Repository navigation
Commit e4471e6
fix(lint)!: refuse a field-level predicate that reads through a reference field at objectstack validate (#20185)
Fixes #20078
Clause-②: no
## What changes
`objectstack validate` (and `build` / `lint`, which run the same
authoring rule) now refuse a field-level `requiredWhen` /
`readonlyWhen`, or a select option's `visibleWhen`, that reads THROUGH a
reference field (`lookup` / `master_detail` / `user` / `tree`) —
`record.account.tier`. The refusal is an `expression-invalid` error
located at the slot (`object 'O' · field 'F' requiredWhen`, `…
readonlyWhen`, `… option 'V' visibleWhen`), naming the reference path
and the related column(s), with the repair that is true for the root the
predicate reads:
- `record.REF.COL` — "Express the check as a `validations[]` `script`
rule, or read a column this object declares." These are the same words
the runtime's own refusal of this fault carries
(`unevaluableFieldRuleError` in `rule-validator.ts`), so the author
meets one repair at both ends.
- `previous.REF.COL` — no seam hydrates `previous`, a validation rule
included, so the repair is to compare the reference itself or read an
own column.
- `parent.REF.COL` on a master-detail line item (judged against the
master's declared field types) — no validation rule binds `parent`, so
the repair is a column the master declares.
This is remedy A as triage routed it (comment 5825661201). Remedy B,
hydrating the field level, is not attempted. The runtime is untouched.
| File | Change |
|---|---|
| `packages/lint/src/validate-expressions.ts` | `refuseFieldTraversal`
in the field walk (requiredWhen, readonlyWhen, option visibleWhen);
`fieldTraversalMessage`; `singleMasterOf` |
| `packages/lint/src/validate-expressions.test.ts` | 18 cases: 10
refusals (each slot, each root, the null-safe spellings, one error per
reference) and 8 controls |
| `packages/cli/test/validate-field-predicate-traversal.test.ts` | the
validate door, spawned (integration project, queue tier): a traversing
config exits 1 with exactly three `expression-invalid` errors carrying
the prescription; a control config exits 0 |
| `packages/spec/src/data/field.zod.ts` | `.describe()` and TSDoc of the
three slots state the rule |
|
`packages/spec/src/migrations/entries/semantic/18.field-predicate-reference-traversal-refused.ts`
+ regenerated `registry.ts` | ADR-0087 semantic entry |
| `content/docs/references/**` | regenerated by `gen:docs`, not
hand-edited |
| `.changeset/20078-field-predicate-reference-traversal-refused.md` |
`@objectstack/lint` + `@objectstack/spec` minor, declared breaking (`!`
and banner), disposition `registered` |
## The dispatch's mechanism hypotheses, measured
- **H1 — confirmed at the door.** On the unfixed tree (lint dist built
from `49144fc` sources; `grep -c` of the new code in
`packages/lint/dist/index.js` = 0), `objectstack validate` over a config
carrying all three traversing predicates printed `Validation passed`,
exit 0. With the fix the same config exits 1 (the CLI test above).
- **H2 — the shared judgment exists; nothing on the runtime side
moved.** The runtime names a field-rule traversal fault with
`@objectstack/formula`'s `analyzeRelationshipTraversals` plus the
reference arbiter (`referenceTargetOf` over `REFERENCE_VALUE_TYPES`).
The lint pass composes the same two: `analyzeRelationshipTraversals`
imported from `@objectstack/formula`, `REFERENCE_VALUE_TYPES` from
`@objectstack/spec/data` (the set `validateExpression`'s own
traversal-conflict arm asks). The runtime has no pre-evaluation
field-level check to import; it detects the fault after evaluation. No
`domain:engine` file is touched.
- **H3 — the `current_user` option shapes stay accepted.** Only `record`
/ `previous` / `parent` member chains are analysed, so
`current_user.can('trav_order', 'edit') && 'admin' in
current_user.positions` on an option is a pinned control (lint unit test
and the CLI control config), green before and after.
- **H4 — the route is `registered`.** The rule text: a narrowing of a
published accept set is a breaking change
(`scripts/pm/clause2-line.mjs`, AGENTS.md Post-Task step 3), and a
declared-breaking changeset must carry an ADR-0087 disposition
(`check-adr-0087-registration.mjs`). The candidates:
`no-migration-prescription` is refused by the gate when the changeset
carries a rewrite recipe, and this one must (the fix is a rewrite into a
`validations[]` rule), so claiming it would rest on a detector miss;
`registered` gives deployed metadata a structured TODO in the step-18
semantic list, which is what triage asked for (「按 ADR-0087
的规则登记这条收窄,写更新说明」). No D2 conversion: turning a field predicate into a
validation rule inverts it into a failure condition and changes form
behaviour, which is the author's judgement. Gate reading: `✓
check-adr-0087-registration: 1 declared-breaking changeset(s) …
[BREAKING+bang] registered field-predicate-reference-traversal-refused
(new here: field-predicate-reference-traversal-refused)`.
- **Census (in-repo half).** Through the lint rule itself: platform
objects — 50 objects, 0 field-predicate slots present, 0 refusals,
positive control (one injected traversal) fired 1;
`examples/app-showcase` objects — 24 objects, 12 field-predicate slots
present, 0 refusals. Through the validate door: `app-todo`, `app-crm`,
`app-multi-package` valid, 0 errors. `app-showcase` could not load at
the door locally (`connector-mcp` dist not built), hence the
object-level reading above. Deployed metadata: NOT MEASURED.
## The runtime behaviour the messages state (engine probe, scratch file,
not committed)
In-memory driver, objectql at base source:
| Shape | Outcome |
|---|---|
| `requiredWhen: record.account.tier == 'enterprise'` — insert, and an
unrelated update | both REFUSED, `rule_violation` / `unevaluable`,
"reads 'tier' through 'account'" |
| `readonlyWhen: record.account.tier == 'gold'` — update writing the
field / update not writing it / insert | REFUSED / accepted / accepted |
| option `visibleWhen: record.account.tier == 'enterprise'` picked with
a `gold` and an `enterprise` account | both ACCEPTED (fail-open, gate
never enforced) |
| `requiredWhen: previous.account.tier == 'gold'` — update | REFUSED |
| `requiredWhen: parent.account.tier == 'gold'` on a detail line —
insert | REFUSED |
| the prescribed rewrites: script `record.account.tier == 'enterprise'
&& (record.po_number == null or empty)`; script with `events:
['update']` and `record.account.tier == 'gold' && record.discount !=
previous.discount` | refused exactly on the failure rows, accepted
otherwise |
## Verification — HEAD `55f3d0a800`
- `pnpm --filter @objectstack/lint test` — 4162 passed; `pnpm --filter
@objectstack/lint typecheck` — exit 0.
- `pnpm --filter @objectstack/cli exec vitest run --project integration
test/validate-field-predicate-traversal.test.ts` — 2 passed. `--project
unit test/vitest-tiers-partition.test.ts` — 22 passed. `pnpm --filter
@objectstack/cli typecheck` — exit 0. The rest of the CLI unit and
integration tiers: not run locally, declared to CI.
- `pnpm --filter @objectstack/spec exec vitest run --project local
src/migrations src/data` — 3307 passed, 2 todo (a declared narrowing of
the spec suite); `pnpm --filter @objectstack/spec typecheck` — exit 0;
`pnpm --filter @objectstack/spec check:generated` — all 15 artifacts up
to date.
- `packages/objectql/src/engine-field-predicate-fault.test.ts`
(read-only control) — 13 passed; the runtime refusal is unchanged.
- `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--ran …` — `112 derived famil(ies) accounted for — 110 run, 2
NOT-MEASURED`. NOT MEASURED: `check:dual-build-cjs-loads` (exit 3,
PREREQUISITE NOT MET: unrelated packages have no local dist);
`check:type-check-debt` (its re-measure leg rebuilds package closures;
the one local run hit my 400s wrapper and was killed; repo-wide ratchet,
CI-owned). Every other family exit 0.
- ESLint, narrowed and proven: the 6 changed TypeScript files, `eslint
--no-inline-config --format json`: 6 files, 0 errors, 0 warnings. The
config never enables type-aware linting (no `parserOptions.project`, per
`eslint.config.mjs`), so this diff cannot move a verdict on an untouched
file. Repo-wide `pnpm lint` is CI's.
## Reverse verification
`node scripts/ablation-replace.mjs` on `validate-expressions.ts`, anchor
`for (const { root, types, owner } of holders) {` → `… of [] as
FieldTraversalHolder[]) {` (anchor 1 → 0, blob `42952246fad7` →
`15165f37c26b`), then the lint test file: **10 failed, 327 passed** —
exactly the 10 refusal cases; all 8 controls stayed green. Restored:
blob == HEAD (`42952246fad7`), `git diff HEAD` empty; re-run 337 passed.
The lint tests import the rule from `src`, so no build sits between the
mutation and the reading. The door's before/after is H1 above: the
unfixed dist passed the config, the fixed dist refuses it.
## Acceptance notes
- **`Clause-②` arm.** The claim's line is `Clause-②: no` with no arm,
copied verbatim above. The change narrows `objectstack validate`'s
accept set, which `clause2-line.mjs` spells `no (narrowing)`;
breaking-ness is carried in the changeset by the `!` summary and the
`**BREAKING**` banner instead, and the ADR-0087 gate reads it
(`[BREAKING+bang]`).
- **Runtime wording for `previous` / `parent` traversals** (same family,
not filed, carrier: none). The probe shows the runtime refusing them
with the generic sentence "reads 'tier', which this object does not
declare — fix the rule's condition, or declare the field", the repair
the runtime's own docblock calls wrong for a traversal; its specific
sentence recognises the `record` root only. With this PR authoring
refuses both with the right repair, so the runtime sentence is reached
only by metadata that never passes the authoring rule.
`rule-validator.ts` is `domain:engine` and outside this claim.
- **Stored objects are not re-judged.** This rule's runtime publish-gate
surface covers flow / action / hook writes, not object writes, so an
object saved through Studio, REST or `sys_metadata` keeps being refused
at write time exactly as before.
- **Coverage boundary.** Injected system columns (`owner_id`,
`created_by`, …) carry no type in the lint index, so a read through one
is not refused (a missed finding, never a false one). Field-level
`visibleWhen`, form-view option `visibleWhen` and action-param option
`visibleWhen` are outside the three slots triage named and are not
judged.
- **Shared describe.** `FormSelectOptionSchema` reuses
`SelectOptionSchema.visibleWhen` by reference, so the option's
`.describe()` also renders on the form-view face; the added sentence is
scoped to "an OBJECT field's option" so it stays true there. The
pre-existing sentence ("The one VISIBILITY predicate the SERVER also
enforces") renders on the form-view face too; not measured here.
- **Not merged with `main`.** Base `49144fc`; `origin/main` has moved
(including new step-18 registry entries). A driver-less `git merge-tree`
probe (bare shared clone, no `merge.os-regen` driver) merged clean.
`dispatch-gates.mjs` flagged the tree as behind `origin/main` with its
own family files changed there, so the family list above is this tree's.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 98f722a commit e4471e6
11 files changed
Lines changed: 759 additions & 25 deletions
File tree
- .changeset
- content/docs/references
- data
- system
- ui
- packages
- cli/test
- lint/src
- spec/src
- data
- migrations
- entries/semantic
Lines changed: 43 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
106 | | - | |
107 | | - | |
| 106 | + | |
| 107 | + | |
108 | 108 | | |
109 | 109 | | |
110 | 110 | | |
| |||
194 | 194 | | |
195 | 195 | | |
196 | 196 | | |
197 | | - | |
| 197 | + | |
198 | 198 | | |
199 | 199 | | |
200 | 200 | | |
| |||
406 | 406 | | |
407 | 407 | | |
408 | 408 | | |
409 | | - | |
| 409 | + | |
410 | 410 | | |
411 | 411 | | |
412 | 412 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
265 | 265 | | |
266 | 266 | | |
267 | 267 | | |
268 | | - | |
269 | | - | |
| 268 | + | |
| 269 | + | |
270 | 270 | | |
271 | 271 | | |
272 | 272 | | |
| |||
597 | 597 | | |
598 | 598 | | |
599 | 599 | | |
600 | | - | |
601 | | - | |
| 600 | + | |
| 601 | + | |
602 | 602 | | |
603 | 603 | | |
604 | 604 | | |
| |||
0 commit comments