Repository navigation
fix(app-shell): a CEL comprehension macro's iteration variable is bound inside its macro, not flagged as out of scope (objectui#10538) - #10619
Conversation
…nd inside its macro, not flagged as out of scope (objectui#10538) findUnknownRefs read every root identifier in a CEL source as a flow variable reference, so `r` in `rows.map(r, r.subject)` drew the "not in scope" warning, including on the spec's own assignment-value example. rootIdentifiers now collects the bindings of map / filter / exists / all / exists_one: the first argument, bound between the macro's `(` and its matching `)`. The same name outside that span (after the close, or as a different macro's receiver) is still a root. Pins beside the existing tests cover the canonical example, each macro, a nested macro, and the negative legs. Claude-Session: https://claude.ai/code/session_01KUxVUa7e39aNjhkKi1gsoy Co-authored-by: Claude <noreply@anthropic.com>
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract reviewServed-tier: Reviewed from git only: ① Derived judgmentsBinding text. Triage Behaviour changes, each at the head:
Adversarial sources at head, all as CEL semantics require: whitespace before the paren, multi-line arguments, nested parens in the body, chained macros ( Asked and missing: nothing. Five macros bound (test Done and not asked: the header bullet at Existing pins edited or deleted: none. The test file is additions only ( New pins can fail. Lit control, my run of the merge-base source against the eight new ② Semver levelChangeset Changeset prose, sentence by sentence:
Docs and README prose added by the diff: none besides the in-file JSDoc. ③ Boundary flags
Implemented-by: VERDICT: PASS |
2 participants
Fixes #10538
Clause-②: no
Implemented by a dispatched dev run of the
domain:uiseat 5, sessionhttps://claude.ai/code/session_01KUxVUa7e39aNjhkKi1gsoy.What changes
findUnknownRefsinpackages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.tsread every root identifier in a CEL source as a flow variable reference. So a comprehension macro's own iteration variable (rinrows.map(r, r.subject)) drew the amber "not in scope" warning. That included the assignment-value example in@objectstack/spec,joinNonEmpty(rows.map(r, r.subject), "\n"), in theAssignmentValueJSDoc.rootIdentifiersnow collects the bindings ofmap,filter,exists,allandexists_one, through a new private helpermacroBindingsin the same file. For a member call.MACRO(v, ...)whose first argument is a bare identifier,vis bound between that call's(and its matching), and nowhere else. Scope is lexical: the receiver, and the same name after the closing), are still roots. A nested macro binds each variable in its own span.findUnknownRefsitself is unchanged. The warning stays advisory; nothing about what blocks Save changes.Why the local scanner and not the parser-backed reader (H2, measured)
@objectstack/formula17.4.0, already an app-shell dependency, exportscollectCelRootIdentifiers. Measured on the five macros, it reports the bound variable too:collectCelRootIdentifiers(source).rootsrows.map(r, r.subject)["rows","r"]rows.filter(x, x.done).size()["rows","x"]rows.exists(e, e.a > 1)["rows","e"]rows.all(a, a.ok)["rows","a"]rows.exists_one(o, o.id == 1)["rows","o"]rows.map(r, r.items.filter(i, i.ok))["rows","r","i"]Switching
findUnknownRefsto that reader would keep this defect. So the fix stays in the local regex scanner, scoped to each macro's argument span, and no parser-backed path is added. The upstream walker has the same defect and its consumers inherit it. That is reported to the dispatching seat as a separate finding (see Acceptance notes). objectstack is not edited here.Pins
A new
describeinflow-ref-check.test.ts, "findUnknownRefs (CEL comprehension macros bind their iteration variable) (objectui#10538)":[];map) with its own bound variable reports[];[];rows.map(r, r.subject) + rflags the trailingr; an inner variable used after its own macro closes is flagged; a bound variable used as a different macro's receiver is flagged;rowz.map(r, r.subject)flagsrowzwith suggestionrows; a member call that is not one of the five (rows.join(sep, other)) binds nothing.Base vs fix, single file (
pnpm exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/flow-ref-check.test.ts, through the shared verify lock):21d34d5(git diff --quietexit 0), new pins added: exit 1,Tests 6 failed | 15 passed (21). The canonical pin fails withexpected [ { token: 'r', …(1) } ] to deeply equal [].5225195): exit 0,Tests 21 passed (21).Two of the negative legs pass on the base as well; they guard against the fix binding too much.
Gates (at head
9fd5fcc, after mergingorigin/mainadeecd6)pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/metadata-admin/inspectors/ packages/app-shell/src/views/metadata-admin/previews/flow-expr-problems.test.ts: exit 0,Test Files 98 passed (98),Tests 1357 passed | 1 skipped (1358). The skip is an existing spec-versionskipIfinflow-node-config.spec-reconciliation.test.ts. The previews test is added becauseflow-expr-problems.tsis the one caller offindUnknownRefsoutside the inspectors folder.pnpm turbo run type-check --filter @object-ui/app-shell --concurrency=2: exit 0,Tasks: 30 successful, 30 total.tsc -p tsconfig.test.json --listFilesOnlylistsflow-ref-check.test.ts, so the pins are type-checked.pnpm exec eslinton the two changed source files: exit 0, no output.pnpm check:control-bytes,check:test-path-roots,check:vi-mock-specifiers,check:vi-mock-inherit,check:vi-mock-override-shape,check:new-line-citations(0 new citations),check:changeset-claims,check:pending-changeset-literals,changeset:check,node scripts/check-changeset-presence.mjsandnode scripts/check-changeset-overwrite.mjs: all exit 0.Changeset:
.changeset/10538-cel-macro-bound-variable.md,'@object-ui/app-shell': patch.Acceptance notes
collectCelRootIdentifiersin@objectstack/formulareports a macro-bound variable as a root, so its closed-root consumers refuse valid CEL. Probe on@objectstack/lint17.4.0:validateApprovalApproverson an approval node whoseexpressionapprover isvars.list.map(x, x.id)gives errorapproval-expression-invalid, "expression approver referencesx". The controlvars.listgives no such error.cel.bind(x, rows, x.size())evaluates upstream (ExpressionEngine.evaluategives{ ok: true, value: 3 }), and this scanner flagscelandxon it, before and after this change.cel.bindis outside the five macros this card rules on, and its shape differs: the variable is bound in the third argument only, and the receivercelis a namespace constant. So it is reported to the seat, not changed here.content/docsdescribes this designer warning, so there is no docs change.Generated by Claude Code