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
11 changes: 11 additions & 0 deletions .changeset/16403-picker-reader-position-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@objectstack/lint": patch
---

`filter-preset-comparand` enters its `publicPicker` object-binding reader by SCHEMA POSITION rather than by key name, so a node that merely spells `publicPicker` no longer takes its whole filter subtree out of the field-typed arm (#16403).

`bindAncestors` walked out through a filter's ancestors and matched `if (key === PUBLIC_PICKER_KEY)` on the property NAME. `walkAuthoredFilters`/`scanForFilters` recognise a filter by key at ANY depth on all eight scanned collections, so that reader was reachable from any node named `publicPicker`, anywhere. Its unresolvable exit is `undefined` — no bound object, so arm 2's field-type oracle answers `false` for every key and the subtree is judged by nobody.

- **No live defect today**: `publicPicker` is declared exactly once as a schema key, on `FormFieldBaseSchema` (`packages/spec/src/ui/view.zod.ts`), and there the reader is correct. What changed is the failure mode the day a second schema declares the same name: it would have inherited this branch silently. Under-reporting is this rule's only permitted failure direction, so the hole would never have VIOLATED that invariant — it would have quietly spent it, where no test asking "was the invariant violated?" could see it.
- **The guard is on the entry, not the exits**: the branch now requires the enclosing ancestor to be a form field (`field`, required on `FormFieldBaseSchema`) — the same read the branch already had to make one line later, so no new coupling between the lint package and the form-view schema. Two of the three exits `#16106`'s review pinned are verbatim untouched: the `picker.object` override (`if (override) return override;`) and both `undefined` legs of the `reference` resolution (`if (!formObject) return undefined;` and the `verdict?.kind === 'ok' ? … : undefined` tail). The third — `!formField` returning `undefined` — is DELETED, and deleting it IS the fix: outside the declared position that line was the silent exit this card is about, while inside the declared position it is unreachable by construction (the guard holding means `formField` is truthy). So the behaviour P3's QUIET pin holds did not move.
- **The `#16106` B1 false refusal stays closed**, measured: a form field's picker filter over a referenced `select` column that shares its name with a parent `date` column still reports nothing, and the positive control — the same filter where the REFERENCED object declares the field as a `date` — still reports at `views[0].sections[0].fields[0].publicPicker.filter[0].value`.
37 changes: 37 additions & 0 deletions packages/lint/src/validate-preset-comparands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,43 @@ describe('validatePresetComparands — arm 2, the FIELD-TYPED equality / members
])).map((f) => f.path)).toEqual(['views[0].sections[0].fields[0].publicPicker.filter[0].value']);
});

// [#16403] The picker reader is entered by POSITION, not by key NAME.
// `scanForFilters` recognises a filter by key at ANY depth on all eight
// surfaces, so a reader keyed on the NAME `publicPicker` alone would be
// handed every future node that happens to spell it — and its unresolvable
// exit is `undefined`, which takes the whole filter subtree out of arm 2
// SILENTLY. That is not a violation of "under-report only"; it is that
// budget being spent where no invariant test can see it.
const outsidePosition = (picker: Record<string, unknown>) => ({
objects: [
{ name: 'crm_opportunity', fields: { close_date: { type: 'date' } } },
{ name: 'crm_contact', fields: { close_date: { type: 'date' } } },
],
dashboards: [{
name: 'sales',
// A widget is NOT a form field: it declares no `field`, so nothing here
// is a `FormFieldPublicPickerSchema` block whatever the key is called.
widgets: [{ id: 'w', object: 'crm_opportunity', publicPicker: picker }],
}],
});

it('[#16403] a `publicPicker` key outside the declared form-field position is bound by the ordinary readers, not by the picker reader', () => {
// Before the position guard this returned [] — the picker reader claimed
// the node on its key, found no enclosing `field`, and left through the
// `undefined` exit.
expect(validatePresetComparands(outsidePosition({ filter: { close_date: 'last_30_days' } }))
.map((f) => f.path)).toEqual(['dashboards[0].widgets[0].publicPicker.filter.close_date']);
// An `object` on the node still names the bound object — the picker
// reader's override and the ordinary `r.object` reader agree, so this half
// is unchanged by the guard.
expect(validatePresetComparands(outsidePosition({ object: 'crm_contact', filter: { close_date: 'last_30_days' } }))
.map((f) => f.path)).toEqual(['dashboards[0].widgets[0].publicPicker.filter.close_date']);
// And the arm still says nothing where the bound object makes it silent —
// the guard restores ordinary binding, it does not force a finding.
expect(validatePresetComparands(outsidePosition({ object: 'no_such_object', filter: { close_date: 'last_30_days' } })))
.toEqual([]);
});

it('keeps arm 1 field-agnostic: an ordering preset still fires with NO objects in the stack, and on a text column', () => {
const findings = validatePresetComparands({
objects: crmObjects,
Expand Down
27 changes: 23 additions & 4 deletions packages/lint/src/validate-preset-comparands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ import { indexObjectGraph, recordsOf, resolveFieldPath, type ObjectGraph } from
* picker (`FormFieldPublicPickerSchema`) queries the REFERENCED object, so
* its `filter` must never fall through to the parent form object (#16106
* review finding B1: that fall-through was a false refusal wherever the two
* objects share a field name with differing types);
* objects share a field name with differing types). That reader claims the
* position by POSITION, not by key name: only where the enclosing record is
* a form field (`field`, required on `FormFieldBaseSchema`). A node that
* merely spells the same key somewhere else is bound by the ordinary
* readers below instead of inheriting this one's unjudged exit (#16403);
* - and, under `objects`, the object itself — its list views, tabs and
* `relatedListFilter` (the filter runs over the CHILD rows, i.e. the object
* that owns the field).
Expand Down Expand Up @@ -420,11 +424,26 @@ function bindAncestors(
// parent and the referenced object share a field name with differing
// types (a `date` on the parent, a `select` whose option value is a
// preset name on the referenced object).
if (key === PUBLIC_PICKER_KEY) {
//
// [#16403] The branch is entered by POSITION, never by key NAME alone.
// `publicPicker` is declared in exactly ONE place — `FormFieldBaseSchema`
// (`ui/view.zod.ts`), where the enclosing record is a form field and its
// `field` is REQUIRED — so the enclosing `field` identifies the position,
// and it is the same read the branch already has to make. Matching on the
// key alone would hand this reader every future node that happens to spell
// `publicPicker`, at any depth on any of the eight surfaces, because
// `scanForFilters` recognises a filter by key rather than by declared
// carrier; such a node would leave through this reader's `undefined` exit
// and take its whole filter subtree out of arm 2 SILENTLY. Under-reporting
// is the only failure direction this arm may have, so that hole could
// never VIOLATE the invariant — it would quietly spend it, where no test
// asking "was the invariant violated?" can see it. Outside the declared
// position the node falls through to the ordinary nearest-ancestor readers
// below, exactly like every other key this walk does not recognise.
const formField = key === PUBLIC_PICKER_KEY ? strName(chain[i - 1]?.node.field) : undefined;
if (formField) {
const override = literalObjectName(r.object);
if (override) return override;
const formField = strName(chain[i - 1]?.node.field);
if (!formField) return undefined;
const formObject = bindAncestors(collection, chain, i - 2, datasets, graph);
if (!formObject) return undefined;
const verdict = resolveFieldPath(graph, formObject, formField);
Expand Down
Loading