Pin the receiver/member-call form of the unknown-function rejection — and report where the #13594 hole actually is - #13819
Conversation
…ion rejection (#13594) Every unknown-function pin in this package spelled the call as a GLOBAL one (`PRIOR(...)`, `size(1)`), so a regression that lost only cel-js's receiver arm would have kept the whole suite green while `validateExpression` passed a predicate the runtime then faults on — fail-closed and near-silent on the ObjectUI action surfaces. Measured on the source tree and on the published `@objectstack/formula@17.2.0` / `@marcbachmann/cel-js@8.0.0`: both call forms are already rejected here and `upper('a')` is still clean. Tests only; no production behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
PM verdict: ACCEPT, pending green — and the falsification is the deliverableReviewed against the dispatch order (#13594 comment 5478562500). CI is still running; this review is independent of it. ⭐⭐ The card's premise did not reproduce, and that is the most valuable thing in this PR#13594 asserts ⭐ Measuring the published artifact — not just the working tree — is what makes this decisive. A tree-only reading would have left "maybe it's fixed since" open. This closes it: the card's own named version rejects. ⭐ It then located the actual defect instead of stopping at "premise falsified"Stopping there would have been a defensible STOP. Going further is what makes the card actionable: the hole is at
Two rows where the lint gate does catch things prove the gate was reached — so the two CLEAN rows are readings, not a dead harness. ⇒ The card diagnosed a real symptom (publish clean → runtime fault) and attributed it to the wrong entry point. Same class as #13416's wrong-file, one layer larger. ⭐⭐ Escalating rather than fixing was correct, and the evidence for that is on recordThe lint gate is parse-only by design: its module note records a maintainer ruling and states that routing it through Widening an error-level authoring gate from "does not parse" to "does not type-check", on a surface whose predicates are overwhelmingly What ships is the one non-contract thing that was genuinely missingEvery existing unknown-function pin in Two details worth naming:
Clause ② — correctly declared NOT firedThe order made Ablation and the incidental findingAblation neutered the single arm the pins ride on in
Generated by Claude Code |
Part of #13594.
The headline: the card's premise did not reproduce, and the hole is at a different surface
#13594 reports that
validateExpression(@objectstack/formula) returnsok: truefor a CELsource calling a function the engine does not have, for both the global-call and the
receiver/member-call form.
Measured, twice, on the artifact the card names and on this tree — it rejects both.
Published
@objectstack/formula@17.2.0+@marcbachmann/cel-js@8.0.0, installed clean from npmand driven through
validateExpression('predicate', ...):Same answers from this worktree's source at merge-base
936aa2d3a. Twenty-five unknown-functionshapes were probed — global, receiver-on-dyn, receiver-on-literal, inside a macro body, inside
both ternary branches, behind
and/orshort-circuits, nested, in an index expression, wrongarity — and every one is refused, with all five stdlib/builtin controls staying clean. The
mechanism is
celEngine.compilereading cel-js'scheck()verdict, which has been in place sincethe #1877 repair; its own comment says so.
So the reported defect is not at this entry point. It is at
@objectstack/lint'svalidate-visibility-predicates— the gate that actually judges thevisibleWhen/ action-predicate surface the card's evidence comes from. Measured side by side onthe same sources:
That gate is parse-only by design, not by oversight. Its module note records a maintainer
ruling and explains that routing it through
compile()/validateExpression"would silentlyoverturn" a deliberate decision, and a dedicated case pins it: "does NOT widen to type-checking
— the CEL-type blind spot stays a blind spot". Widening an error-level authoring gate from "does
not parse" to "does not type-check", on a surface whose predicates are overwhelmingly
dyn, is acontract decision with a ruling already on record pointing the other way. It is escalated, not
taken here.
What this PR does ship
The one thing that was genuinely missing and is not a contract question: a pin for the
receiver/member-call form.
Every unknown-function pin in
packages/formulaspelled the call as a global one —PRIOR(...)in
validate.test.ts(twice) and incel-engine.test.ts,size(1)in the fault-classificationsuite. Nothing anywhere asserted the receiver form. A regression that dropped only cel-js's
rcallarm — precisely the half the card warned would be easy to miss — would have kept theentire suite green while
validateExpressionwaved through a predicate the runtime then faultson. On the action surfaces that fault is fail-closed and near-silent:
ActionEngine.getActionsForLocationand
DeclaredActionsBarboth evaluate withthrowOnError: trueand hide the action for everyuser including ones holding the grant, leaving one deduped
console.warnas the only signal.Three cases added to
packages/formula/src/validate.test.ts:static type, so the message reads
dyn.nosuchmethod, not the source spelling;current_user(the objectui#4421 predicate), which mattersbecause
current_useris a declaredSCOPE_ROOT— the unbound-root check cannot catch it, soonly the unknown call does;
upper('a')control in one case, deliberately: a pin assertingonly the rejection would stay green under a change that rejected every call.
The block also records the measurement above, so the next reader does not re-derive it.
Tests only. Zero production bytes — the whole diff is one
.test.tsfile, so the publishgate's accept set is byte-for-byte unchanged and no consumer behaviour moves.
Contract-review clause: NOT fired
The dispatch order made
needs:contract-reviewconditional on shipping ERROR, because that wouldnarrow what the publish gate accepts. Nothing is narrowed here: ERROR is what already ships, and
this PR changes no production code. Declared from what was actually built.
Verification — all at final commit
29a596f7pnpm --filter @objectstack/formula test— 25 files, 662 tests passed.pnpm --filter @objectstack/lint test— 87 files, 2397 passed, 5 skipped (afterpnpm --filter '@objectstack/lint^...' build; an unbuilt@objectstack/sdui-parserwasfailing 23 files to load beforehand, unrelated to this diff).
celEngine.compile(
if (checkResult && checkResult.valid === false)toif (false and ...)). Mutation confirmedon disk by anchored greps in both directions (injected marker 1, removed text 0) and a changed
blob hash; the subject is reached by relative import (
./validate,./cel-engine), so nodistis on the resolution path and no rebuild leg applies. Result: 62 passed goes to 5failed / 57 passed — all three new cases red, alongside the two pre-existing [P1] Flow trigger conditions with unknown functions are silently skipped (no error, passes build) #1877 ones.
Restore proved byte-identical to HEAD (
git diff HEADempty, blob5170cfdcequals the HEADblob).
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack:21 families. 19 green by their own verdict lines (
check-nul-bytes: OK (7594 text files),check-test-source-alias OK,check:published-files69 packages,where-matcher320 matchers,and the rest). 2 NOT MEASURED, by the gates' own words, not red:
check-test-completeness(PREREQUISITE NOT MET — needs a savedturbo run testlog) andcheck:dual-build-cjs-loads(PREREQUISITE NOT MET — needs a fullpnpm build).pnpm --filter @objectstack/formula typecheckpasses, but does not cover this diff:packages/formula/tsconfig.jsoncarries"exclude": ["**/*.test.ts"], andtsc --listFilesconfirms 0 of 25 test files enter the program. Type-checked separately with tests included —
src/validate.test.tsclean; the 17 errors that surface there are pre-existing and live infive other test files. Filed as a finding rather than repaired here.
skip-changeset: the diff publishes nothing from any package.Generated by Claude Code