From e2e5e1ad95614733b4b00c3769c06b59bf939ee1 Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Wed, 16 Sep 2026 19:30:07 +0000 Subject: [PATCH 1/2] fix(lint): enter the publicPicker object reader by schema position, not by key name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bindAncestors` in `validate-preset-comparands.ts` matched the public-lookup picker reader on the property NAME `publicPicker`. `scanForFilters` recognises a filter by key at any depth on all eight scanned collections, so that reader was reachable from any node spelling the same name — and its unresolvable exit is `undefined`, which leaves the whole filter subtree unbound and therefore unjudged by the field-typed arm, silently. The branch now requires the enclosing ancestor to be a form field (`field`, required on `FormFieldBaseSchema`) — the same read the branch already made one line later, so no new coupling. Outside that position a node falls through to the ordinary nearest-ancestor readers like any other unrecognised key. The three exits the #16106 review pinned are unchanged. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk --- .../16403-picker-reader-position-guard.md | 11 ++++++ .../src/validate-preset-comparands.test.ts | 37 +++++++++++++++++++ .../lint/src/validate-preset-comparands.ts | 27 ++++++++++++-- 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 .changeset/16403-picker-reader-position-guard.md diff --git a/.changeset/16403-picker-reader-position-guard.md b/.changeset/16403-picker-reader-position-guard.md new file mode 100644 index 00000000000..3775f25a129 --- /dev/null +++ b/.changeset/16403-picker-reader-position-guard.md @@ -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. The three exits `#16106`'s review pinned (the `picker.object` override, the `reference` resolution, and the `undefined` no-fall-through) are byte-for-byte unchanged. +- **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`. diff --git a/packages/lint/src/validate-preset-comparands.test.ts b/packages/lint/src/validate-preset-comparands.test.ts index 7e797dc3363..0c78a34a91a 100644 --- a/packages/lint/src/validate-preset-comparands.test.ts +++ b/packages/lint/src/validate-preset-comparands.test.ts @@ -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) => ({ + 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, diff --git a/packages/lint/src/validate-preset-comparands.ts b/packages/lint/src/validate-preset-comparands.ts index 21a38572272..ed5acbe83af 100644 --- a/packages/lint/src/validate-preset-comparands.ts +++ b/packages/lint/src/validate-preset-comparands.ts @@ -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). @@ -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); From dffc5b965fe7ef8fd1ccfcad94a6d2dd05efb68f Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Wed, 16 Sep 2026 20:02:52 +0000 Subject: [PATCH 2/2] docs(changeset): state which picker exit moved instead of claiming all three held MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The diff deletes `if (!formField) return undefined;`, so "the three exits are byte-for-byte unchanged" was inaccurate in text that ships to CHANGELOG.md. Two exits are verbatim untouched; the third is deleted, and deleting it is the fix — outside the declared position it was the silent exit, inside it it is unreachable by construction. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk --- .changeset/16403-picker-reader-position-guard.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/16403-picker-reader-position-guard.md b/.changeset/16403-picker-reader-position-guard.md index 3775f25a129..8080ad87504 100644 --- a/.changeset/16403-picker-reader-position-guard.md +++ b/.changeset/16403-picker-reader-position-guard.md @@ -7,5 +7,5 @@ `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. The three exits `#16106`'s review pinned (the `picker.object` override, the `reference` resolution, and the `undefined` no-fall-through) are byte-for-byte unchanged. +- **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`.