Skip to content

Commit 1e496f9

Browse files
fix(lint): enter the publicPicker object reader by schema position, not by key name (#18524)
Fixes #16403 Clause-②: no ## What changed `bindAncestors` (`packages/lint/src/validate-preset-comparands.ts`) matched the public-lookup picker reader on the property NAME: ```ts if (key === PUBLIC_PICKER_KEY) { ``` `walkAuthoredFilters` / `scanForFilters` recognise a filter by KEY at any depth on all eight scanned collections, so that reader was reachable from any node spelling `publicPicker`, at any depth, on any surface — and its unresolvable exit is `undefined`, i.e. no bound object, so arm 2's field-type oracle answers `false` for every key and the whole filter subtree is judged by nobody. The branch is now entered by SCHEMA POSITION: ```ts const formField = key === PUBLIC_PICKER_KEY ? strName(chain[i - 1]?.node.field) : undefined; if (formField) { ``` `publicPicker` is declared exactly once as a schema key — `FormFieldBaseSchema` in `packages/spec/src/ui/view.zod.ts`, where the enclosing record is a form field whose `field` is REQUIRED. So the enclosing `field` identifies the position, and it is the same read the branch already had to make one line later. Outside that position the node falls through to the ordinary nearest-ancestor readers, exactly like every other key this walk does not recognise. ## Coupling: which size was chosen, and what it costs The triage authorised either a comment alone or a `chain[i-1].node.field` guard, and asked for the cost of the choice to be stated. **The guard was chosen**, for one reason: it introduces **no new coupling** between `packages/lint` and the form-view schema. The read it adds at the entry is byte-for-byte the read the branch already performed at its second line — the guard moves it, it does not invent it. A comment alone would have left the rule's behaviour dependent on a key NAME staying globally unique, which is not a property any gate holds. What it costs, stated plainly: - The guard reaches only ONE case: a node reached under `publicPicker` whose enclosing ancestor declares no string `field`, AND which declares no `object` of its own. Every other shape is unchanged (an `object` on the node is read by the picker reader's override and by the ordinary `r.object` reader alike, to the same value). - In that one case the node is now bound by the ordinary nearest-ancestor readers instead of leaving unbound. That is the rule's general contract — the same contract every other unrecognised key in the config already runs under — rather than a name-triggered exemption from it. It is not a new class of risk; it is the removal of a name-shaped exception to the existing one. - A malformed form field (one that omits the required `field`) therefore also falls through. `FormFieldBaseSchema` requires `field`, so such metadata is refused at the schema door; this rule is not the door that holds it. ## No live defect today — and why the gap was still worth closing `publicPicker` has exactly one schema declaration today, and there the reader is correct. What the guard changes is the failure mode the day a second schema declares the same name: that schema's filters would have inherited this branch silently. Under-reporting is this rule's only permitted failure direction, so the hole could never have VIOLATED the invariant — it would have quietly spent it, where no test asking "was the invariant violated?" can see it. ## Evidence **Negative control (triage item 3) — the #16106 B1 false refusal stays closed.** The one real picker still enters the branch and still resolves to the referenced object. Measured, `pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/validate-preset-comparands.test.ts` on the fixed tree: `Test Files 1 passed (1) · Tests 22 passed (22)`, both B1 tests among them — - `[B1] stays QUIET on a publicPicker filter over a referenced select column that shares its name with a parent date column` — PASS (a form field's picker filter over `crm_account.close_date: select` reports nothing, where the parent `crm_opportunity.close_date` is a `date`). - `[B1] POSITIVE CONTROL: the same picker filter is still refused when the REFERENCED object declares the field as a date` — PASS, reporting at `views[0].sections[0].fields[0].publicPicker.filter[0].value` through the field's `reference` and through the `object` override alike. **Reverse verification.** The fix was committed first; the guard was then reverted in place, on disk, and the same suite re-run. Direction: RED, exactly one test. ``` pre-mutation : guarded-entry lines 1 · name-only-entry lines 0 post-mutation : guarded-entry lines 0 · name-only-entry lines 1 · mutated blob != HEAD blob ablated run : Tests 1 failed | 21 passed (22) FAIL [#16403] a `publicPicker` key outside the declared form-field position … - Expected ["dashboards[0].widgets[0].publicPicker.filter.close_date"] + Received [] restore : git diff HEAD empty · porcelain clean · blob == HEAD blob · guarded-entry lines back to 1 ``` The received `[]` is the gap itself: the picker reader claimed the node on its key, found no enclosing `field`, and left through the `undefined` exit, taking the subtree out of arm 2 without a word. The two B1 tests stayed GREEN under the ablation, which is the other half of the reading — the guard is orthogonal to the behaviour #16347 fixed. **Package verification.** `pnpm --filter @objectstack/lint test` — `Test Files 103 passed (103) · Tests 3845 passed (3845)`. `pnpm --filter @objectstack/lint typecheck` — exit 0. `pnpm lint` (repo-wide `eslint . --no-inline-config`) — exit 0, whole population, no narrowing claimed. ## 验收备注 (Acceptance notes) The triage ruling (`5579122478`), quoted verbatim — Chinese decisions are not translated: > 1. 项 (1)(属性名匹配)与项 (2)(lint 跟了一跳 route 不跟)**可分开落地**,⛔ 不必同一个 PR。**项 (1) 是本卡主项**。 > 2. ⛔ **不动 picker 分支的 `undefined` 出口** —— 它被 #16106 复审第三条消融腿(P3)的 QUIET pin 钉住,且是有意的。⭐ 分诊把修法的形状定死了:**出口被钉住了,入口没有** ⇒ 要加的是「什么样的节点算 picker」的约束。 > 3. **若加 `chain[i-1].node.field` 守卫**:**阴性对照必测** —— `view.zod.ts` 上那唯一一个真 picker 在守卫之后**仍然**进入 picker 分支并解析到被引用对象。⭐ 这正是 PR #16347 修好的那条 B1 **假拒绝**(表单字段的 `publicPicker.filter` 被拿父表单的对象去判),⛔ 不得被这次加固重新打开。 > 4. **若只加注释**:注释必须说清三件 —— ① 按属性名匹配、② 今天只有一处声明、③ 再出现同名键时会**静默**进入本分支。⛔ 不许只写"注意"。 > 5. **项 (2) 若要动**:先给一次读数 —— 今天有没有真实的 dotted picker path 被 `filter-preset-comparand` 报过?⛔ 两个方向都要写。⚠️ 且 ⛔ **不要顺手改成"在这条规则里多报一条路径错误"** —— 那会让 `filter-preset-comparand` 开始判它名字之外的东西,正是项 (1) 担心的范围蔓延。 > 6. **#16340 ⛔ 不合并进来**(同为漏报,机制不同)。 Disposition, item by item: 1. Item (1) only. Item (2) is untouched here. 2. TWO of the three exits 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 by this diff, 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, since the guard holding means `formField` is truthy. So the behaviour P3's QUIET pin holds did not move, and nothing but the entry condition changed semantically. (An earlier revision of this body and of the changeset said "all three byte-for-byte unchanged"; that was wrong against the diff and is corrected here and in `.changeset/16403-picker-reader-position-guard.md`.) 3. Measured above, not asserted. Both B1 tests pass on the fixed tree and pass under the ablation. 4. The guard carries the comment as well: the docblock and the in-branch note state the name-matching shape, the single declaration, and the silent inheritance. 5. Item (2) is not addressed here, so no reading is owed and none is claimed. `filter-preset-comparand` still reports only what its own name covers. 6. Out of scope: #16340 is not merged in and is not touched. ### Noted, not filed - `check:cross-package-test-inputs` reds on a checkout where `packages/spec/dist/` has been built, at `origin/main`, with this diff absent — and stays green where it has not. Two-leg control taken at BASE `97233b90c` in a detached control worktree: without `packages/spec/dist/` present, exit 0 (`OK: 29 package(s) read outside themselves, all declared …`); with the identical `dist` tree hard-linked in and nothing else changed, exit 1 with the same FAIL text this branch produced (`@objectstack/cli descends a directory tree from packages/spec/dist/`). `lint.yml`'s `lint` job installs but never builds, so the walked-root leg of that gate cannot fire in CI at all. Reported to the dispatching seat for triage; it is outside this card's file surface and is not touched here. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: claude[bot] <noreply@anthropic.com>
1 parent 2496415 commit 1e496f9

3 files changed

Lines changed: 71 additions & 4 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/lint": patch
3+
---
4+
5+
`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).
6+
7+
`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.
8+
9+
- **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.
10+
- **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.
11+
- **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`.

‎packages/lint/src/validate-preset-comparands.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,43 @@ describe('validatePresetComparands — arm 2, the FIELD-TYPED equality / members
621621
])).map((f) => f.path)).toEqual(['views[0].sections[0].fields[0].publicPicker.filter[0].value']);
622622
});
623623

624+
// [#16403] The picker reader is entered by POSITION, not by key NAME.
625+
// `scanForFilters` recognises a filter by key at ANY depth on all eight
626+
// surfaces, so a reader keyed on the NAME `publicPicker` alone would be
627+
// handed every future node that happens to spell it — and its unresolvable
628+
// exit is `undefined`, which takes the whole filter subtree out of arm 2
629+
// SILENTLY. That is not a violation of "under-report only"; it is that
630+
// budget being spent where no invariant test can see it.
631+
const outsidePosition = (picker: Record<string, unknown>) => ({
632+
objects: [
633+
{ name: 'crm_opportunity', fields: { close_date: { type: 'date' } } },
634+
{ name: 'crm_contact', fields: { close_date: { type: 'date' } } },
635+
],
636+
dashboards: [{
637+
name: 'sales',
638+
// A widget is NOT a form field: it declares no `field`, so nothing here
639+
// is a `FormFieldPublicPickerSchema` block whatever the key is called.
640+
widgets: [{ id: 'w', object: 'crm_opportunity', publicPicker: picker }],
641+
}],
642+
});
643+
644+
it('[#16403] a `publicPicker` key outside the declared form-field position is bound by the ordinary readers, not by the picker reader', () => {
645+
// Before the position guard this returned [] — the picker reader claimed
646+
// the node on its key, found no enclosing `field`, and left through the
647+
// `undefined` exit.
648+
expect(validatePresetComparands(outsidePosition({ filter: { close_date: 'last_30_days' } }))
649+
.map((f) => f.path)).toEqual(['dashboards[0].widgets[0].publicPicker.filter.close_date']);
650+
// An `object` on the node still names the bound object — the picker
651+
// reader's override and the ordinary `r.object` reader agree, so this half
652+
// is unchanged by the guard.
653+
expect(validatePresetComparands(outsidePosition({ object: 'crm_contact', filter: { close_date: 'last_30_days' } }))
654+
.map((f) => f.path)).toEqual(['dashboards[0].widgets[0].publicPicker.filter.close_date']);
655+
// And the arm still says nothing where the bound object makes it silent —
656+
// the guard restores ordinary binding, it does not force a finding.
657+
expect(validatePresetComparands(outsidePosition({ object: 'no_such_object', filter: { close_date: 'last_30_days' } })))
658+
.toEqual([]);
659+
});
660+
624661
it('keeps arm 1 field-agnostic: an ordering preset still fires with NO objects in the stack, and on a text column', () => {
625662
const findings = validatePresetComparands({
626663
objects: crmObjects,

‎packages/lint/src/validate-preset-comparands.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,11 @@ import { indexObjectGraph, recordsOf, resolveFieldPath, type ObjectGraph } from
120120
* picker (`FormFieldPublicPickerSchema`) queries the REFERENCED object, so
121121
* its `filter` must never fall through to the parent form object (#16106
122122
* review finding B1: that fall-through was a false refusal wherever the two
123-
* objects share a field name with differing types);
123+
* objects share a field name with differing types). That reader claims the
124+
* position by POSITION, not by key name: only where the enclosing record is
125+
* a form field (`field`, required on `FormFieldBaseSchema`). A node that
126+
* merely spells the same key somewhere else is bound by the ordinary
127+
* readers below instead of inheriting this one's unjudged exit (#16403);
124128
* - and, under `objects`, the object itself — its list views, tabs and
125129
* `relatedListFilter` (the filter runs over the CHILD rows, i.e. the object
126130
* that owns the field).
@@ -420,11 +424,26 @@ function bindAncestors(
420424
// parent and the referenced object share a field name with differing
421425
// types (a `date` on the parent, a `select` whose option value is a
422426
// preset name on the referenced object).
423-
if (key === PUBLIC_PICKER_KEY) {
427+
//
428+
// [#16403] The branch is entered by POSITION, never by key NAME alone.
429+
// `publicPicker` is declared in exactly ONE place — `FormFieldBaseSchema`
430+
// (`ui/view.zod.ts`), where the enclosing record is a form field and its
431+
// `field` is REQUIRED — so the enclosing `field` identifies the position,
432+
// and it is the same read the branch already has to make. Matching on the
433+
// key alone would hand this reader every future node that happens to spell
434+
// `publicPicker`, at any depth on any of the eight surfaces, because
435+
// `scanForFilters` recognises a filter by key rather than by declared
436+
// carrier; such a node would leave through this reader's `undefined` exit
437+
// and take its whole filter subtree out of arm 2 SILENTLY. Under-reporting
438+
// is the only failure direction this arm may have, so that hole could
439+
// never VIOLATE the invariant — it would quietly spend it, where no test
440+
// asking "was the invariant violated?" can see it. Outside the declared
441+
// position the node falls through to the ordinary nearest-ancestor readers
442+
// below, exactly like every other key this walk does not recognise.
443+
const formField = key === PUBLIC_PICKER_KEY ? strName(chain[i - 1]?.node.field) : undefined;
444+
if (formField) {
424445
const override = literalObjectName(r.object);
425446
if (override) return override;
426-
const formField = strName(chain[i - 1]?.node.field);
427-
if (!formField) return undefined;
428447
const formObject = bindAncestors(collection, chain, i - 2, datasets, graph);
429448
if (!formObject) return undefined;
430449
const verdict = resolveFieldPath(graph, formObject, formField);

0 commit comments

Comments
 (0)