Skip to content

fix(lint): enter the publicPicker object reader by schema position, not by key name - #18524

Merged
os-try-charles merged 2 commits into
mainfrom
claude/issue-16403-picker-key-position
Sep 16, 2026
Merged

os-try-charles merged 2 commits into
mainfrom
claude/issue-16403-picker-key-position

Conversation

@os-try-charles

@os-try-charles os-try-charles commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #16403

Clause-②: no

What changed

bindAncestors (packages/lint/src/validate-preset-comparands.ts) matched the public-lookup picker reader on the property NAME:

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:

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 出口 —— 它被 spec/lint: a date-range PRESET name is refused only under ordering operators — the same preset on the same date field passes as a bare / $eq / $in comparand #16106 复审第三条消融腿(P3)的 QUIET pin 钉住,且是有意的。⭐ 分诊把修法的形状定死了:出口被钉住了,入口没有 ⇒ 要加的是「什么样的节点算 picker」的约束。
  3. 若加 chain[i-1].node.field 守卫:阴性对照必测 —— view.zod.ts 上那唯一一个真 picker 在守卫之后仍然进入 picker 分支并解析到被引用对象。⭐ 这正是 PR feat(lint): field-typed equality/membership arm for filter-preset-comparand on declared date/datetime fields #16347 修好的那条 B1 假拒绝(表单字段的 publicPicker.filter 被拿父表单的对象去判),⛔ 不得被这次加固重新打开。
  4. 若只加注释:注释必须说清三件 —— ① 按属性名匹配、② 今天只有一处声明、③ 再出现同名键时会静默进入本分支。⛔ 不许只写"注意"。
  5. 项 (2) 若要动:先给一次读数 —— 今天有没有真实的 dotted picker path 被 filter-preset-comparand 报过?⛔ 两个方向都要写。⚠️ 且 ⛔ 不要顺手改成"在这条规则里多报一条路径错误" —— 那会让 filter-preset-comparand 开始判它名字之外的东西,正是项 (1) 担心的范围蔓延。
  6. lint: a field-typed rule cannot see registry-injected temporal columns (created_at / updated_at) — the object graph carries their names, not their types #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: lint: a field-typed rule cannot see registry-injected temporal columns (created_at / updated_at) — the object graph carries their names, not their types #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

…ot by key name

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tests tooling labels Sep 16, 2026
@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 4 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2882528786ee33c870a9c14ee0c4d81030250c76 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6aae8b74a8a7bbd8fd2876e5ed5172fc5a462efd — the merge of head dffc5b965fe7ef8fd1ccfcad94a6d2dd05efb68f into base 2882528786ee33c870a9c14ee0c4d81030250c76, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6aae8b74a8a7bbd8fd2876e5ed5172fc5a462efd && git checkout 6aae8b74a8a7bbd8fd2876e5ed5172fc5a462efd
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2882528786ee33c870a9c14ee0c4d81030250c76 dffc5b965fe7ef8fd1ccfcad94a6d2dd05efb68f && git checkout -B drift-repro 2882528786ee33c870a9c14ee0c4d81030250c76 && git merge --no-ff dffc5b965fe7ef8fd1ccfcad94a6d2dd05efb68f

node scripts/docs-audit/affected-docs.mjs --json 2882528786ee33c870a9c14ee0c4d81030250c76

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

…l three held

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
@os-try-charles
os-try-charles marked this pull request as ready for review September 16, 2026 20:22
@os-try-charles
os-try-charles added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 1e496f9 Sep 16, 2026
42 of 43 checks passed
@os-try-charles
os-try-charles deleted the claude/issue-16403-picker-key-position branch September 16, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tests tooling

Projects

None yet

2 participants