fix(formula): the ADR-0032 §1c retry rewrites only the operands that faulted (#7098)

[os-zhuang]

Closes #7098.

The defect

hydrateOverloadStrings's docblock in packages/formula/src/cel-engine.ts claimed:

Used only on the isNumericOverloadError retry path, so it can never change a comparison that already evaluated cleanly — it only rescues one that already faulted.

That claim was false, and it was load-bearing: it was the stated reason the hydration was allowed to be unconditional and scope-wide. The retry knows only that the whole expression faulted, not that each sub-comparison did.

Both of the filer's reproductions confirmed on this branch's base before any edit:

record.n >= 4 && record.s == "5.0"        with { n: "7", s: "5.0" }  ->  { ok: true, value: false }
record.n >= 4 ? record.s == "5.0" : false with { n: "7", s: "5.0" }  ->  { ok: true, value: false }

record.n >= 4 faults and is correctly rescued. But record.s was hydrated to the number 5 too, so the author's deliberate string equality — true in evaluation 1 — became 5 == "5.0", which CEL answers false across types. Silently wrong: { ok: true }, no fault, no log line, no red test.

The reach measurement, taken before the shape was chosen

Does this evaluation path reach an RLS predicate? No.

Row-level security compiles its using / check predicates through compileCelToFilter — SQL pushdown on the read side, and matchesFilterCondition against the post-image on the write side (plugin-security/security-plugin.ts step 3.6). Declared sharing rules do the same (plugin-sharing/bootstrap-declared-sharing-rules.ts). Neither calls celEngine.evaluate, which is the only home of this retry. No access-control decision could be inverted by this defect.

But the PM's binary was a false dichotomy, and shape 2 is still unavailable. The dispatch framed "No ⇒ confined to display / derived formulas ⇒ shape 2 becomes genuinely arguable". It is not so confined. celEngine.evaluate's non-test consumers are:

consumer	what a flipped boolean does
objectql/validation/rule-validator.ts:1649 checkPredicate	a validation rule's failure condition — reads as not violated, so an invalid write is accepted
objectql/validation/rule-validator.ts:1808 checkConditional	a when conditional runs the wrong branch
objectql/validation/rule-validator.ts:588 isReadonlyWhenLocked	a field the author locked becomes writable
objectql/hook-wrappers.ts:308	a declared hook silently does not run
service-automation/engine.ts:5443	flow start / step conditions
objectql/engine.ts:723, :2346	formula fields and default values (the display / derived ones)

Validation rules are fail-closed on a fault (#4649) — but a silently flipped boolean is not a fault, so fail-closed never engages. Documenting a write-integrity gate that silently stops rejecting is no more acceptable than documenting an inverted access-control boolean; only one of the two is access control. Shape 2 (correct the docblock, accept the behaviour) was therefore not taken.

The fix — shape 1, per operand position

The coercion is now per operand position. The scope is never rewritten; the faulting operand is wrapped in double(…) / date(…) in place, on the retry path only. This is the discipline rewriteTemporalEquality already documents two functions above ("no field-wide trade-off") — so this is not novel design, it brings the hydration in line with the convention its neighbour already states — and one step stricter, since it is per position rather than per field.

The filing carried forward that the fault message carries operand types, not a path into the scope, which is what makes this more than a three-line change. That is correct, and the fix does not try to read the operand out of the message. It reads the AST — the same parse → walk → serialize machinery the two existing rewrites in this file use — and resolves each operand against the scope values already in hand. An operand is rewritten only when all three hold:

1. the operator raises on a string-versus-number/Timestamp pair rather than answering one, so the comparison cannot have produced an answer to change;
2. the counterpart is a number or Timestamp in this scope — read off the values, not off a static type, since every field is dyn under unlistedVariablesAreDyn;
3. the operand's own value is a §1c serialization artifact — an entirely-numeric string or an ISO-8601 date. "02134" and free text still fault loudly.

Together these make the docblock's guarantee true by construction rather than by assertion.

The measurement that sharpened the fix

Measured per operator on cel-js 8.0.0, against an int literal, a number-valued field, a today() Timestamp and a Date-valued field:

operators	string vs number / Timestamp	eligible?
< <= > >= + - * / %	fault — no such overload: dyn<string> >= int	yes
== != in	answer — false / true	no

CEL equality is total, so record.s == 5 over { s: "5" } is a clean false, not a fault. This is the root of the defect: the string equality in the reproduction never faulted at all, so it always had an answer, and the scope-wide hydration overruled it. An earlier draft of this fix included ==/!=/in and was wrong for exactly that reason; the measurement caught it.

Field.date strings not matching a Timestamp under == stays owned by rewriteTemporalEquality (#3183), which wraps them statically on the clean path where both sides are known from the source.

What changes answer, stated plainly

Only expressions that already reached the §1c retry. Within those, the ones that also contain a string equality/inequality, a string membership test, the same field compared both as a number and as a string, or a numeric-looking string the expression returns rather than compares — record.n >= 4 ? record.s : "none" returned the number 5 and now returns "5.0", so a text Field.formula was storing a different value than the record held.

One class becomes a loud fault where it was silently rescued: an operand whose value the walk cannot read before deciding — bound by a comprehension (record.items.exists(i, i.price > 100)) or behind a computed index. That is the deliberate trade. Silently rescuing an operand we cannot prove faulted is the defect being closed, so those report the original no such overload unchanged in shape and message.

Reverse-verification

The new test file was run against the unmodified pre-fix source in a separate origin/main worktree, with red/green predicted per case first.

Predicted 12 red / 11 green. Actual 9 red / 14 green. All 9 actual reds were predicted. The three deviations were all predicted-red-but-green, and have one cause: CEL's && error absorption. error && false is false, not an error, so in

record.n >= 4 && record.n == 7     ("7" == 7 is false)
record.n >= 4 && record.n in [1,7] ("7" in [1,7] is false)
record.n >= 4 && record.s != "5.0" ("5.0" != "5.0" is false)

the fault is absorbed, the retry never armed, and there was nothing for the scope-wide hydration to corrupt. My prediction had assumed the retry armed in every && case. Verified by measuring each pre-fix directly. These rows are pinned anyway — they are the cases that must keep answering the same beside a faulting compare, and post-fix they do.

Gates

Enumerated fresh from origin/main: 64 in lint.yml. No packages/spec file is touched, so the 9 spec-filtered gates and the #6017 cross-seat declaration do not apply.

gate / suite	result
@objectstack/formula tests	607 passed (584 pre-existing + 23 new), 24 files
@objectstack/objectql tests	2849 passed, 165 files
@objectstack/service-automation tests	885 passed, 72 files
@objectstack/plugin-security tests	878 passed, 43 files
pnpm lint	clean
pnpm --filter @objectstack/formula typecheck	clean
pnpm check:engine-double-contract	OK — 126 pinned, 133 debt, 2 exempt
pnpm --filter @objectstack/lint check:doc-formula-expressions	OK — 22 record-scoped examples over 387 files judged clean
pnpm check:empty-changeset	OK
pnpm check:changeset-gate-self-tests	OK
pnpm check:type-check-coverage	OK — 63/77 packages
pnpm build	71/71 tasks

CI is the authority; only a job conclusion of completed: success counts as green, and this stays draft until it is.

Notes for the next card

parseCelToAstWithReason is untouched — signature, return shape and behaviour are all unchanged. #7073, which consumes it, is unblocked by this landing and needs no adjustment for it.

Related

#7098 · #6679 / PR #7097 (the trigger side of the same retry — narrows when it arms, not what it rewrites; this reproduces independently of it) · #1530 / #1534 (why the retry exists) · #4649 (validation fail-closed) · #3183 (rewriteTemporalEquality, the per-occurrence precedent) · ADR-0032 §1c.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 10, 2026 1:36am

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/formula.

4 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/data-modeling/formulas.mdx (via @objectstack/formula)
• content/docs/data-modeling/validation.mdx (via @objectstack/formula)
• content/docs/plugins/packages.mdx (via @objectstack/formula)
• content/docs/protocol/objectui/record-alert.mdx (via @objectstack/formula)

⛔ 2 release-owned page(s) also reference the affected code. These are read-only:

• content/docs/releases/v15.mdx (via @objectstack/formula)
• content/docs/releases/v16.mdx (via @objectstack/formula)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.