Repository navigation
Commit d9c4cdc
fix(lint)!: refuse a page variable
domain:server#5 · os-project-manager · session_01FEkQ5oc56xChD6Wrc4cGSt
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XUmUy7mddyM9KUzene3eqMA)_
Fixes #28725
Clause-②: no (narrowing)
Metadata-protocol: narrowed
Skills-docs: none — no file under `skills/**` teaches a page variable's
`source` binding or `page.NAME` reads; both refusals carry their own fix
and `os explain` entries.
## 维护者速读
- **改了什么**:页面变量的 `source` 写了页面上不存在的组件 id(如 `no_such_component`),以及组件
`visibleWhen` 读了页面没声明的变量(如 `page.nosuch == 'x'`),现在都被拒:保存、草稿诊断、发布、`os
validate` 四处同一条结论,定位到 `variables[i].source` 或该组件。
- **为什么改**:以前这两种都能保存、草稿显示 valid、照常发布。控制台按组件 id 精确匹配写入方,`source`
写错就没有任何组件会写这个变量;`page` 里只有声明过的变量,读没声明的名字永远取不到值,组件就无条件显示。作者毫不知情。
- **风险与回滚**:低。只收紧写入口,已存页面照常加载和服务,只有下次保存才被判。仓内语料 3 个 `source`、5 个
`page.X` 读取(都在 showcase 两个页面),零命中;examples 三个 `os validate` 全过。回滚即撤回
`@objectstack/lint` 这次改动。
-
**先看**:`packages/lint/src/validate-visibility-predicates.ts`(`firstUndeclaredPageRead`
与 checkElement 的 3c
段);`packages/lint/src/validate-page-variables.ts`(`source`
判定);`packages/lint/src/runtime-gate.page-variable-references.test.ts`。
- **两分钟验证**:`pnpm --filter @objectstack/lint exec vitest run
src/runtime-gate.page-variable-references.test.ts`,预期 17/17 通过。
- **需要你做的**:无,席位落地。
## What changes
- `packages/lint/src/validate-page-variables.ts`: a fourth id beside
#28663's three, `page-variable-source-unknown`. A variable whose
`source` names a component `id` the page does not hold is refused,
located at `variables[i].source`. The page's component ids come from
`walkPageComponents`, the one traversal every page rule shares (regions,
slots, and the sub-trees inside `properties`), read once per page and
only when some entry has a `source`; no second walk. A source-authored
page (`kind: 'react'` / `'html'`) is not judged on `source`: its
components live in the code, which that walk deliberately skips.
- `packages/lint/src/validate-visibility-predicates.ts`: a new gating
id, `visibility-page-variable-undeclared`. On a page component, a
`visibleWhen` reading `page.NAME` or `page.?NAME` where the page
declares no variable `NAME` is refused, located at the component, naming
the first such read. Judged from the AST the `has()` check already holds
(formula's `parseCelToAst`, no second parser), with the page's declared
names passed into `checkElement` from the pages loop only. The nav
walker is untouched (#28726 owns it). Stand-downs are missed catches
only: an index read (`page['x']`), and any predicate whose comprehension
binds `page` as its loop variable (`COMPREHENSION_MACROS`, reused from
`predicate-rhs-position.ts`).
- Why that file and not `validate-predicate-path-refs.ts` (measured, as
the order asked): the path rule resolves `data.*` against a metadata
type's schema only; nothing in it reads a `page` root. The visibility
rule is where `page` is declared as a root (`VIEW_PAGE_EXTRA_ROOTS`) and
where the page walk already hands each component's predicate to the
parser.
- `packages/lint/src/index.ts`: exports both id constants.
`packages/lint/src/rule-explanations.ts`: two `os explain` entries.
- No `authoring-rules.ts` change: both ids live inside rules already
registered on `page` writes, so save, draft `_diagnostics`, publish and
`os validate` carry them with no registry or `metadata-protocol` source
change, and the CLI rule count does not move (no `content/docs/**`
edit).
- `.changeset/28725-page-variable-source-and-read.md`:
`@objectstack/lint` minor, BREAKING banner, both declaration lines,
ADR-0087 `not-required (no-migration-prescription)`.
## Why
#28663 judged the `variables` list itself; the contract review of PR
#28723 (finding S3) left these two references admitted, and this card
measured both on a stock showcase: draft `_diagnostics` `valid: true`,
publish succeeds. What the console does with each, read at `ui/` on this
head:
- **`source`.** `usePageVariableBinding(componentId)`
(`ui/packages/react/src/hooks/usePageVariables.tsx`) returns
`definitions.find((d) => d.source === componentId)`, and the two writer
elements (`element:record_picker`, `element:text_input`) call it with
their own `schema.id`. A `source` no component carries matches nothing,
so no element writes the variable and it keeps its default. The refusal
matches a binding that can never resolve.
- **`page.NAME`.** `buildNodeEvaluationScope`
(`ui/packages/react/src/SchemaRenderer.tsx`) sets `page: pageVariables`,
written after the ambient spread, so nothing else can supply `page`. The
variables come from `PageVariablesProvider`, mounted by the page
renderer only when the page declares variables and seeded from the
declarations alone. Its only writers are a bound element (by a
declaration's `source`) and the dashboard filter bar (its own declared
filters), so an undeclared name is never present. A page with no
variables gets the frozen empty fallback. No route params or other
bindings reach `page`. The read faults and the element renders, which is
the refusal's stated consequence.
- Nesting: a provider merges an outer scope (objectui#2578), documented
for a filtered dashboard embedded in a page. That is a dashboard reading
the page's variables, not a page component reading another scope's, so
it does not reach this rule.
**Migration note.** Shipped corpus (`examples/**`, `packages/*/src`,
platform apps): 3 variable `source` bindings and 5 `page.NAME` predicate
reads, all on two showcase pages (`page-variables.page.ts`,
`contact-form.page.ts`). **0 refused.** `os validate` at head on
`examples/app-showcase`, `app-crm`, `app-todo`: `"valid": true`, 0
findings of the new ids. Positive controls on the same command: planting
`source: 'project_pickr'` or `visibleWhen: "page.selectedProjectID ==
''"` in the showcase page makes `os validate` exit 1 with the new id at
the right path. Stored pages keep loading and serving; only the next
save of one with a finding is refused.
## Tests
- `packages/lint/src/runtime-gate.page-variable-references.test.ts`
(new): the card's `source: "no_such_component"`, a case-mismatched id,
the card's `page.nosuch == 'x'`, a `{ dialect, source }` envelope,
`page.?nosuch`, and a `page.X` read on a page with no variables, each
refused at the page door with its path, with door / `os validate`
(`runAuthoringRules('validate')`) parity. Controls admitted at both: the
showcase picker shape, a variable with no `source`, a `has()`-guarded
declared read, a read through a declared `object` variable, a
comprehension binding `page`, no `page` read. Also: a writer nested in a
container's `children` resolves; a `kind: 'react'` page is not judged on
`source`; two undeclared reads are one finding; both faults are two
findings.
-
`packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts`:
two new rows in the #26707 table (the draft saves, reads `valid: false`
with the publish's words, publish refuses `422 INVALID_METADATA`); the
clean controls read `valid: true` and publish; paths are item-relative
(`variables.0.source`, `regions.0.components.0`).
-
`packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts`:
the active `saveMetaItem` refuses each (door message = build message,
nothing stored); the clean page lands.
- Fixture triage (declaration added, not respelled):
`validate-visibility-predicates.test.ts` and
`runtime-gate.page-visibility-predicate-writes.test.ts` each had a clean
control reading `page.selectedId` / `page.selectedProjectId` on a page
that declared no variables. Each page now declares the variable; the
second file's family id set gains the new id, so its controls also
assert it stays silent.
<details><summary>Red on base 8bceb6a, green on head</summary>
Base = the four edited lint src files written from 8bceb6a, lint
rebuilt (`ablation-dist-preflight --absent` exit 0), then restored (`git
diff HEAD` empty) and rebuilt.
```
base lint runtime-gate.page-variable-references.test.ts Tests 9 failed | 8 passed (17) (the 8 = the controls)
base protocol draft-publish-verdict + runtime-authoring-gate Tests 5 failed | 255 passed (260)
head lint Tests 17 passed (17) · protocol Tests 260 passed (260)
```
Record: `pm-orders/server/evidence/28725/redbase2.log` (script
`redbase.sh`).
</details>
<details><summary>Ablations (scripts/ablation-replace.mjs, lint rebuilt,
ablation-dist-preflight both legs, restore = blob equals HEAD)</summary>
```
SRC (source check behind globalThis.ABL_SRC) lint 4 failed | 283 passed (3 files) · protocol 3 failed | 257 passed
READ (read check behind globalThis.ABL_READ) lint 7 failed | 280 passed (3 files) · protocol 3 failed | 257 passed
each leg: anchor x1 -> x0, marker in dist (preflight exit 0), restore blob == HEAD, git diff HEAD empty; rebuild, preflight --absent exit 0 for both markers
```
Record: `pm-orders/server/evidence/28725/ablate.log` (scripts
`ablate.sh`, `ablate-run.sh`).
</details>
<details><summary>Live probe: both card cases on a stock showcase boot,
base vs head</summary>
Commands: `pnpm --filter @objectstack/example-showcase dev -- --fresh -p
PORT`; `POST /api/v1/auth/sign-in/email` as the seeded admin; `POST
/api/v1/packages` a scratch writable package; `PUT
/api/v1/meta/page/qa_src_page?mode=draft&package=PKG` (a picker
`project_picker`, `variables[0].source: "no_such_component"`); `PUT
/api/v1/meta/page/qa_read_page?mode=draft&package=PKG` (`visibleWhen:
"page.nosuch == 'x'"`, variable declared and sourced from the picker);
`GET …?state=draft&package=PKG` for each; `POST
/api/v1/packages/PKG/publish-drafts`; an active `PUT` of a clean control
and of the read case. Base = 8bceb6a lint sources over the four edited
files, lint rebuilt.
```
base case 1 draft _diagnostics {"valid":true} · case 2 {"valid":true} · publish-drafts "outcome":"published" (2) · active PUT case 2 200
head case 1 _diagnostics valid:false [variables.0.source page-variable-source-unknown]
head case 2 _diagnostics valid:false [regions.0.components.1 visibility-page-variable-undeclared]
head publish-drafts "outcome":"refused" INVALID_METADATA pages[0].variables[0].source · active PUT case 2 422 INVALID_METADATA
both clean control (source = the picker, reads page.selectedProjectId) active PUT 200
```
Record: `pm-orders/server/evidence/28725/live-probe.txt` (`probe.sh`,
`probe-base.sh`).
</details>
## Acceptance notes
- Declaration: the claim's line reads `Clause-②: no`; this body and the
changeset carry `Clause-②: no (narrowing)`, because
`check-changeset-no-major.mjs` refuses `Metadata-protocol: narrowed`
beside a line without the `(narrowing)` arm. The order's Declarations
section names it a narrowing.
- File surface: the claim names `packages/lint/src/**` and its tests;
the two `packages/metadata-protocol` test files above are added for the
draft / publish / active-save proof, as #28723 did. Test-only, no source
change there.
- Serial with #28726: its PR #28756 (the nav over-budget arm in the same
file) landed after this PR opened; this PR merged it and carries the
seat-ordered fold of the two `visibility-predicate-over-budget` sentence
builders (see "Patch round" below).
- Not judged, by design (missed catches, never false errors):
`page['x']` index reads; `{{page.NAME}}` tokens in action bodies (an
interpolation surface, not a predicate the visibility rule walks);
`page.NAME` in props-bag CEL slots (`page:tabs` item `visibleWhen`,
which binds `data` to the row, and the other slots
`validatePagePredicateBounds` reads). Whether `source` names a component
that can WRITE (only `element:record_picker` and `element:text_input`
call the binding) is not judged either; the ruling asks only that the id
exist.
## Verification
Derived gate loop (`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, 59 families) at `d350dda3` (the second
merge of origin/main): 59 exit 0. `dispatch-gates --ran`: `59 derived
famil(ies) accounted for — 59 run, 0 NOT-MEASURED (a DERIVED zero — all
59 recorded an exit code and none of them is 3)`. Record:
`pm-orders/server/28725-gates-ran.txt`.
- Three families first read exit 3 (prerequisite, nothing measured) and
were re-run to exit 0 on the same head: `check-plugin-teardown-shape
--self-test` (shallow clone; `git fetch --depth=1 origin 621a487`),
`check:dual-build-cjs-loads` (seven packages outside the built closure
had no `dist/`; built), `check:type-check-debt` (lock queue-timeout
behind this run's own dogfood battery; re-run: `--re-measure: OK — 0 raw
tsc error(s)`). The same 59 also all read exit 0 at the first merge
`cef4f60f`.
- Package suites at `d350dda3`: `@objectstack/lint` Test Files 188
passed, Tests 7780 passed; `@objectstack/metadata-protocol` Test Files
294 passed / 4 skipped, Tests 22029 passed / 26 skipped; both
`typecheck` exit 0. `eslint packages/lint packages/metadata-protocol
--no-inline-config` exit 0. `check-platform-checklist --changed-from
origin/main` OK.
- Dogfood battery (the `(narrowing)` arm) at `d350dda3`, closure built,
`pnpm --filter @objectstack/dogfood test` under the lock: VERDICT
command-exit 0 · Test Files 374 passed / 3 skipped (377) · Tests 3034
passed / 28 skipped (3062).
- `os validate` at `d350dda3` on `examples/app-showcase`, `app-crm`,
`app-todo`, `app-multi-package`: `"valid": true`, 0 findings of the new
ids (`pm-orders/server/evidence/28725/os-validate.txt`).
- CI owns the repo-wide lint and every family outside the derived set.
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(No `packages/spec` edit.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes (scoped: `eslint
packages/lint packages/metadata-protocol --no-inline-config` exit 0).
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: not touched.
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English (the maintainer brief
excepted, by the dispatch contract).
## Patch round: over-budget builder fold (seat-ordered deviation)
Ordered by server seat 5 after #28756 (PR for #28726) landed:
`checkElement`'s bounds arm and #28756's `navOverBudgetFinding` built
the `visibility-predicate-over-budget` sentence in two places. Head
`f0519ad0` (after merging origin/main `d08b859a`) folds them into one
`overBudgetFinding(refusal, where, path, surface, root)`, the way
`syntaxFinding` already works: the `SyntaxSurface` gains
`overBudgetHint(root)`, so the element surface keeps its `ROOT.f in
[...]` / formula-field remedy (root from the site's layer) and the nav
surface keeps its `current_user.positions.exists(...)` remedy.
`navOverBudgetFinding` is gone.
- Wording byte-identical: 9 over-budget findings (view section and page
component on the runtime layer, a schema-bound form and a page on the
metadata layer, a nav group and its child) from `dist/` built off the
pre-fold blob and off the fold compare `cmp`-identical
(`pm-orders/server/evidence/28725/fold-byte-identity.txt`). The existing
pins (#28726's nav tests, the page/view over-budget tests) pass
unchanged.
- At `f0519ad0`: `@objectstack/lint` Tests 7797 passed (188 files),
typecheck exit 0; `@objectstack/metadata-protocol` draft-publish-verdict
+ runtime-authoring-gate Tests 265 passed, typecheck exit 0; scoped
eslint exit 0. Derived gate loop 59/59 exit 0, `--ran` derived zero
(`pm-orders/server/28725-fold-gates-ran.txt`).
- Merge conflict resolved in `protocol.draft-publish-verdict.test.ts`
(this PR's two #28725 tests and #28756's nav over-budget test sat at the
same spot; both kept).
- Dogfood battery: green at `d350dda3`; NOT MEASURED at `f0519ad0`
(lint-only, byte-identical refactor plus the merge; CI runs it).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01FEkQ5mBAqi5RnPHhcrHgTn
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5mBAqi5RnPHhcrHgTn)_
---------
Co-authored-by: Claude <noreply@anthropic.com>source naming no component on the page, and a page.NAME read the page does not declare (#28769)1 parent 8f294e0 commit d9c4cdc
10 files changed
Lines changed: 534 additions & 59 deletions
File tree
- .changeset
- packages
- lint/src
- metadata-protocol/src
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
279 | 279 | | |
280 | 280 | | |
281 | 281 | | |
| 282 | + | |
282 | 283 | | |
283 | 284 | | |
284 | 285 | | |
| |||
675 | 676 | | |
676 | 677 | | |
677 | 678 | | |
678 | | - | |
| 679 | + | |
| 680 | + | |
679 | 681 | | |
680 | 682 | | |
681 | 683 | | |
682 | 684 | | |
683 | 685 | | |
| 686 | + | |
684 | 687 | | |
685 | 688 | | |
686 | 689 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1014 | 1014 | | |
1015 | 1015 | | |
1016 | 1016 | | |
| 1017 | + | |
| 1018 | + | |
| 1019 | + | |
| 1020 | + | |
| 1021 | + | |
| 1022 | + | |
| 1023 | + | |
| 1024 | + | |
| 1025 | + | |
| 1026 | + | |
| 1027 | + | |
| 1028 | + | |
| 1029 | + | |
| 1030 | + | |
| 1031 | + | |
| 1032 | + | |
1017 | 1033 | | |
1018 | 1034 | | |
1019 | 1035 | | |
| |||
2772 | 2788 | | |
2773 | 2789 | | |
2774 | 2790 | | |
| 2791 | + | |
| 2792 | + | |
| 2793 | + | |
| 2794 | + | |
| 2795 | + | |
| 2796 | + | |
| 2797 | + | |
| 2798 | + | |
| 2799 | + | |
| 2800 | + | |
| 2801 | + | |
| 2802 | + | |
| 2803 | + | |
| 2804 | + | |
| 2805 | + | |
| 2806 | + | |
2775 | 2807 | | |
2776 | 2808 | | |
2777 | 2809 | | |
| |||
5418 | 5450 | | |
5419 | 5451 | | |
5420 | 5452 | | |
| 5453 | + | |
5421 | 5454 | | |
5422 | 5455 | | |
5423 | 5456 | | |
| |||
5480 | 5513 | | |
5481 | 5514 | | |
5482 | 5515 | | |
| 5516 | + | |
5483 | 5517 | | |
5484 | 5518 | | |
5485 | 5519 | | |
| |||
| 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 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| 48 | + | |
48 | 49 | | |
49 | 50 | | |
50 | 51 | | |
| |||
62 | 63 | | |
63 | 64 | | |
64 | 65 | | |
| 66 | + | |
65 | 67 | | |
66 | 68 | | |
67 | 69 | | |
68 | 70 | | |
69 | 71 | | |
70 | 72 | | |
71 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
72 | 78 | | |
73 | 79 | | |
74 | 80 | | |
75 | 81 | | |
| 82 | + | |
76 | 83 | | |
77 | 84 | | |
78 | 85 | | |
| |||
0 commit comments