Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/18550-reference-carrier-residue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
"@objectstack/objectql": minor
"@objectstack/rest": minor
"@objectstack/metadata-protocol": minor
"@objectstack/lint": minor
"@objectstack/verify": minor
---

The remaining raw `FieldSchema.reference` readers now **REFUSE** a carrier they cannot read, instead of answering "no target" (#18550). The previous release routed the arbiter (`referenceCarrierOf`) and the lint target readers; these were the measured residue of the same ruling — every reader, not just the arbiter.

`FieldSchema.reference` is `z.string().optional()`, so `ObjectSchema.safeParse` refuses an object- or array-valued carrier at the contract door. These reads are the other door: the one a value reaches only when it never went through parse — a hand-built fixture, a raw `registerObject`, a stored row rehydrated past its schema.

**`@objectstack/objectql`** — both of the delete cascade's carrier reads (`planCascadeAtomicity` and `cascadeDeleteRelations`). This is the one with a measurable runtime consequence, and it is why the level is not `patch`:

```
before acct=1 task=1
delete RESOLVED true <- success reported to the caller
after acct=0 task=1 <- an ORPHANED master_detail row
```

An unreadable carrier made the relation invisible to the cascade, so the parent was deleted, the detail row stayed, and the caller was told the delete succeeded — no `restrict` refusal, no `set_null`, nothing logged. It now refuses before any row is touched.

**`@objectstack/rest`** — the public-form lookup picker's field-def fallback. The field def is also hoisted out of the metadata fetch's `catch {}`, so an unreadable carrier is no longer reported as `LOOKUP_TARGET_MISSING`: "no target is declared" and "the declared target cannot be read" want different fixes from whoever owns the metadata.

**`@objectstack/metadata-protocol`** — the seed dependency graph, which also retires an `as string` cast that asserted exactly what its truthiness guard had not checked.

**`@objectstack/lint`** — the four remaining target readers: `masterDetailCount` (`validate-expressions`), the `displayField` consumer edge (`validate-field-consumers`), the field and action-param targets (`validate-object-references`), and `masterOf` (`validate-sharing-rule-enforceability`).

**`@objectstack/verify`** — `relationTarget`, which no longer degrades an unreadable carrier to the generic "has no `reference` target" an object with no relationship metadata at all receives.

`null`, `undefined` and `''` are ABSENCE, not a wrong shape, and still answer `undefined` at every one of these sites — a field is allowed to name no target, and `StrictField` declares `reference` nullable. Each site's absence answer is pinned alongside its refusal.

Upgrading: nothing conformant changes. A non-string `reference` could not be authored, stored or parsed before this release either; what changes is that one now fails loudly at the read instead of being read as an absent target. If a test asserted the old silence, assert the refusal instead.
3 changes: 2 additions & 1 deletion content/docs/ui/forms.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ sections: [{
| `displayFields` | Fields projected into each result row (plus `id`); the visitor's `q` is `contains`-matched against the **first** entry. At most 5; omitted → `['name']`. |
| `maxResults` | Rows per request, integer 1–50 (default 20). 50 is a hard server ceiling; there is **no pagination** on this surface (`offset` is pinned to 0), so a leaked endpoint cannot enumerate the table. |
| `filter` | Static pre-filter rows (same `{ field, operator, value }` dialect as list-view filters), ANDed ahead of the visitor's search. |
| `object` | The object to search. Optional — omit it and the server resolves the target from the field's own definition on the parent object: its `reference` key, and only that key. A stored row spelling the target `referenceTo` / `target` / `options.objectName` is **not** resolved — the route answers `500 LOOKUP_TARGET_MISSING` — because `FieldSchema` accepts no spelling but `reference`. Declare it only to search something other than what the field points at. |
| `object` | The object to search. Optional — omit it and the server resolves the target from the field's own definition on the parent object: its `reference` key, and only that key. A stored row spelling the target `referenceTo` / `target` / `options.objectName` is **not** resolved — the route answers `500 LOOKUP_TARGET_MISSING` — because `FieldSchema` accepts no spelling but `reference`. That key is also **read through the one carrier accessor**, so a stored row whose `reference` holds something other than a string (an object, an array) is refused rather than searched — see the error table below. Declare it only to search something other than what the field points at. |

Those four keys are the whole block. It admits exactly what the route enforces
— an unknown subkey, a 6th display field, or `maxResults: 51` is a **parse
Expand Down Expand Up @@ -285,6 +285,7 @@ Errors:
| `403 LOOKUP_NOT_PUBLIC` | the field has no `publicPicker` block — the deliberate loud default (#3022); also any server-managed anchor (`owner_id`, `organization_id`, …), which never gets a picker even if one is declared |
| `404 FORM_NOT_FOUND` | slug not registered on any `sharing.allowAnonymous: true` view |
| `500 LOOKUP_TARGET_MISSING` | the referenced object could not be resolved from either `publicPicker.object` or the field definition — the field names no target object at all (or its object metadata is unreachable). Until #7486 this also fired for a perfectly well-formed field, because the fallback read only the legacy spellings and not the canonical `reference`; declaring `object` was the workaround and is no longer needed. |
| `500 INTERNAL_ERROR` | the field def **declares** a target this route cannot READ — a stored `reference` holding an object or an array rather than the object name `FieldSchema` declares. ⚠️ Deliberately **not** `LOOKUP_TARGET_MISSING`: "nothing names the target" and "the named target is unreadable" want different fixes from whoever owns the metadata, so they get different answers. The unreadable carrier is named in full in the server log (it is withheld from the response body, as every fault's text is); the picker's search never runs. |

### Auth model

Expand Down
59 changes: 59 additions & 0 deletions packages/lint/src/validate-expressions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4234,3 +4234,62 @@ describe("validateStackExpressions — a non-record entry in an object's `fields
expect(issues[0].message).toContain('amount');
});
});

/**
* [#18550] `masterDetailCount` must refuse a `reference` carrier it cannot
* read, rather than counting the relationship as undeclared.
*
* One of the measured residue sites of ruling letter E item 2 on #18095. The
* count was `typeof ref === 'string' && ref.trim() !== ''`, which gives an
* UNREADABLE carrier the same answer as an ABSENT one — so an object whose
* master IS declared counted zero masters, and the `parent`-scope gate above
* reported "declares no `master_detail` relationships" about metadata that
* declares one. A finding about the wrong thing: the relationship is not
* missing, its target is unreadable, and the two want different fixes.
*
* Absence keeps its answer: `undefined`, `null`, `''` and a whitespace-only
* carrier all still count as no master, because none of them names an object.
*/
describe('masterDetailCount — an unreadable `reference` carrier is refused (#18550)', () => {
const lintObject = (obj: Record<string, unknown>) => () => validateStackExpressions({ objects: [obj] });
const detailOn = (carrier: Record<string, unknown>) => ({
name: 'inv_line',
fields: {
inv: { type: 'master_detail', ...carrier },
qty: { type: 'number', readonlyWhen: "parent.status == 'paid'" },
},
});

it('control: a READABLE carrier counts as a master, so the `parent` gate stays silent', () => {
// Without this, every refusal below could pass on a gate that had stopped
// resolving masters at all.
expect(lintObject(detailOn({ reference: 'inv' }))()).toHaveLength(0);
});

it('an OBJECT-valued carrier REFUSES — ⛔ not a "declares no master_detail" finding', () => {
const run = lintObject(detailOn({ reference: { object: 'inv' } }));
expect(run).toThrow(TypeError);
expect(run).toThrow(/validate-expressions masterDetailCount/);
expect(run).toThrow(/`reference` is an object/);
expect(run).toThrow(/FieldSchema declares it as an optional STRING/);
});

it('an ARRAY-valued carrier refuses too, naming the shape it found', () => {
expect(lintObject(detailOn({ reference: ['inv', 'inv2'] }))).toThrow(/`reference` is an array \(length 2\)/);
});

// ── ABSENCE: still counted as no master, still reported as the `parent`-gate
// finding, ⛔ never thrown on. These are the cases a mechanical
// throw-on-falsy sweep would break.
it.each([
['undefined (the key omitted)', {}],
['null (`StrictField` declares it nullable)', { reference: null }],
["'' (names no object)", { reference: '' }],
['whitespace only (names no object either)', { reference: ' ' }],
])('absence stays a FINDING, not a throw: %s', (_label, carrier) => {
const issues = lintObject(detailOn(carrier))();
const parentScope = issues.filter((i) => /reads `parent`/.test(i.message));
expect(parentScope).toHaveLength(1);
expect(parentScope[0]!.message).toMatch(/declares no `master_detail` relationships/);
});
});
19 changes: 17 additions & 2 deletions packages/lint/src/validate-expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ import type { FlowNodeParsed, FlowEdgeParsed } from '@objectstack/spec/automatio
// hand-written notion of "blank" here — that drift is what #15662 built the
// shared refusal to prevent.
import { EvaluatedExpressionInputSchema, EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec/shared';
import { referenceCarrierOf } from '@objectstack/spec/data';

import { collectFlowVariableNames, shadowedFieldReads, shadowedFieldMessage } from './flow-variable-scope.js';
import { injectedColumnsFor, unprovisionedInjectedColumnsFor } from './system-fields.js';
Expand Down Expand Up @@ -377,8 +378,22 @@ function masterDetailCount(obj: AnyRec): number {
// rejected alias — `field.zod.ts:331` maps it to `reference` in the strict
// error map, so a field spelling it does not parse (#5017). See the
// `## Scope` table on this module for why a consumer must not re-admit it.
const ref = def.reference;
if (typeof ref === 'string' && ref.trim() !== '') n += 1;
//
// [#18550] Read through the ONE arbiter. ABSENCE is unchanged and still
// uncounted — `undefined` / `null` / `''` answer `undefined`, and the
// `.trim()` test below still drops a whitespace-only carrier, which names
// no object either. UNREADABILITY used to be uncounted too, and that is
// the silence: an object-valued carrier made a declared `master_detail`
// invisible to this count, so `parent` was judged unbound (or two masters
// read as one) from metadata that does declare a relationship.
// ⭐ The literal `.reference` read STAYS here, in the argument, and only
// the SHAPE judgment moves out — the form `validate-security-posture.ts`
// and `data-model-rules.ts` already use, and for their stated reason: the
// #5017 receiver meta-test reads this rule's SOURCE to prove it reads
// `reference` and never an alias, and a read folded inside a helper call
// would disarm that scan silently.
const ref = referenceCarrierOf({ reference: def.reference }, 'validate-expressions masterDetailCount');
if (ref !== undefined && ref.trim() !== '') n += 1;
}
return n;
}
Expand Down
59 changes: 59 additions & 0 deletions packages/lint/src/validate-field-consumers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -471,3 +471,62 @@ describe('validateFieldConsumers (#15922)', () => {
});
});
});

/**
* [#18550] The `displayField` consumer edge must refuse a `reference` carrier
* it cannot read, rather than recording no edge at all.
*
* One of the measured residue sites of ruling letter E item 2 on #18095. The
* read was `strName(field.reference)`, which answers `undefined` for an
* unreadable carrier exactly as it does for an absent one — so a field a
* lookup DOES display was recorded as consumed by nobody, and this rule then
* reported it as carrier-only. The ledger under-reported, and the finding
* pointed at the displayed field instead of the unreadable carrier.
*
* Absence keeps its answer: with no target there is no object to look a
* `displayField` up on, so no edge is recorded and nothing throws.
*/
describe('validateFieldConsumers — an unreadable `reference` carrier is refused (#18550)', () => {
const stackWith = (carrier: AnyRec): AnyRec => ({
objects: [
{ name: 'crm_account', fields: { name: { type: 'text' }, legal_name: { type: 'text' } } },
{
name: 'crm_contact',
fields: {
name: { type: 'text' },
account: { type: 'lookup', displayField: 'legal_name', ...carrier },
},
},
],
// A consumer root OTHER than `objects` is this rule's entry condition
// (`hasConsumerRoot`): with only `objects` present it returns early and
// never walks a field, so a fixture without one would make every case
// below vacuous.
views: [{ name: 'contact_list', object: 'crm_contact', viewKind: 'list', columns: ['name'] }],
});

it('control: a READABLE carrier records the `displayField` edge, so the target is not carrier-only', () => {
const findings = validateFieldConsumers(stackWith({ reference: 'crm_account' }));
expect(findings.map((f) => f.path)).not.toContain('objects[0].fields.legal_name');
});

it('an OBJECT-valued carrier REFUSES — ⛔ not a silent missing edge', () => {
const run = () => validateFieldConsumers(stackWith({ reference: { object: 'crm_account' } }));
expect(run).toThrow(TypeError);
expect(run).toThrow(/validate-field-consumers walkObject/);
expect(run).toThrow(/`reference` is an object/);
expect(run).toThrow(/FieldSchema declares it as an optional STRING/);
});

it.each([
['undefined (the key omitted)', {}],
['null (`StrictField` declares it nullable)', { reference: null }],
["'' (names no object)", { reference: '' }],
])('absence records no edge and does NOT throw: %s', (_label, carrier) => {
// With no target there is no object to resolve `displayField` against, so
// the displayed field is genuinely unconsumed here — the rule's ordinary
// answer, reached without a throw.
const findings = validateFieldConsumers(stackWith(carrier as AnyRec));
expect(findings.map((f) => f.path)).toContain('objects[0].fields.legal_name');
});
});
10 changes: 9 additions & 1 deletion packages/lint/src/validate-field-consumers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@

import { deriveFieldGroupLayout, resolveDisplayField } from '@objectstack/spec/data';
import type { DisplayNameObjectMeta } from '@objectstack/spec/data';
import { referenceCarrierOf } from '@objectstack/spec/data';
import { collectionEntries } from './collection-entries.js';
import { recordsOf } from './object-graph.js';
import { injectedColumnsFor } from './system-fields.js';
Expand Down Expand Up @@ -549,7 +550,14 @@ function walkObject(ledger: ConsumerLedger, obj: AnyRec, objectName: string, obj
walk(ledger, value, objectName, 'objects', `${objPath}.${key}`, [key], key);
}
for (const { rec: field, path: fieldPath } of collectionEntries(obj.fields, fieldsPath)) {
const reference = strName(field.reference);
// [#18550] The carrier through the ONE arbiter: `strName` answered
// `undefined` for an unreadable one exactly as it does for an absent one,
// so the `displayField` consumer edge below was never recorded and the
// ledger under-reported — a field a lookup DOES display read as unused.
// Absence still answers `undefined` and records nothing.
// Same form as the sibling lint readers: the literal `.reference` read
// stays at the site, only the shape judgment moves to the arbiter.
const reference = referenceCarrierOf({ reference: field.reference }, 'validate-field-consumers walkObject');
const displayField = strName(field.displayField);
if (reference && displayField && ledger.declares(reference, displayField)) {
ledger.record(reference, displayField, { root: 'objects', path: `${fieldPath}.displayField`, kind: 'display' });
Expand Down
Loading
Loading