Skip to content

Commit 4c2cd29

Browse files
fix(lint)!: publish refuses a text-vs-number comparison in a validation rule (or another fail-closed predicate) instead of warning, then refusing every write (#29923)
<!-- claude-projects-attribution: {"github_login":"os-support-ai"} --> _Requested by **Yibo Wang** · [project thread](https://claude.ai/code/project/chan_01BVUyyPie8gHwoPM18iABwB?thread=cmsg_01BVUyyPie8gHwoPM18iABwB7J3TmFTYMYQrfpye6GQNg9)_ Fixes #29904 Clause-②: no (narrowing) — the author-time gate (`os build`, `os validate`, the object save door and the package publish) refuses a `type-mismatch` it used to pass with a warning, at the predicate sites that fail closed. Nothing is widened. Metadata-protocol: narrowed Skills-docs: updated here — `content/docs/data-modeling/formulas.mdx` (the build-time validation table's type-soundness row said `warning` for every site) and the MCP skill text in `packages/mcp/src/skill-md.ts`; `skills/**` teaches no text-vs-number comparison and no type-soundness severity (grep for the verdict words and `record.X > N` on text fields: 0 hits), so no skill or eval moves. ## 维护者速读 - 改了什么:校验规则(以及 requiredWhen / readonlyWhen、选项 visibleWhen、hook 条件)里"文本字段和数字比大小"(如 `record.name > 5`)现在发布时直接拒绝(error),公式字段仍只给 warning。 - 为什么改:这些位置求值失败会拒绝写入;以前发布只提示"会得到 null",发布成功后该对象的每次新建和更新都被拒,一个笔误就让整张表录不进数据。 - 风险与回滚:收窄发布门,已存在的此类规则下次发布会被拒(它们本来就让写入全部失败);回滚即 revert 本 PR,无数据迁移。 - 先看:`packages/formula/src/validate.ts`(fail-closed 分支与文案)、`packages/lint/src/validate-expressions.ts`(`FAIL_CLOSED` 站点表及各调用点)、`packages/mcp/src/mcp-http-tools.ts`(validation 站点)。 - 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run src/publish-type-mismatch-fail-closed.test.ts`,预期 3 条全过:校验规则被拒、公式仍发布带 warning、可空孪生照旧被拒。 - 需要你做的:无,席位落地。 ## What changes - **`@objectstack/formula`.** `ExprSchemaHint` gains `faultOutcome?: 'fail-closed'`. The validator cannot tell a validation rule's `predicate` from a flow condition's, so the caller that knows the site declares it. With it, `type-mismatch` is graded an **error** whose message says what really happens (the write, or the operation a hook guards, is refused) and how to compare like with like; the refusal's `params` carry `outcome: 'fail-closed'`. Without it, the warning and its "evaluates to null" text are unchanged. - **`@objectstack/lint`.** One constant, `FAIL_CLOSED`, with the site ledger in its docblock, passed by `check()` at exactly the fail-closed sites: validation rule `condition` / `when` and nested `then` / `otherwise` rules, field `requiredWhen` / `readonlyWhen`, option `visibleWhen`, hook `condition`. These are the sites where the nullable-comparison twin is already an error with the `'fail-closed'` outcome (plus `readonlyWhen` and option `visibleWhen`, see below). - **`@objectstack/mcp`.** `validate_expression` with `site: 'validation'` passes the same outcome, so the tool an agent self-checks with answers what publish will. Its description and the served skill text say so. - **`@objectstack/spec`.** Upgrade guide entry `validation-type-mismatch-refused` (semantic: no mechanical rewrite). ADR-0087 disposition `registered validation-type-mismatch-refused`. - **Docs.** `content/docs/data-modeling/formulas.mdx`: the type-soundness row's severity and fix. ### Which roles fail closed (measured by reading each evaluator) | site | runtime on a fault | this PR | |:--|:--|:--| | validation rule `condition` / `when`, nested rules | `rule-validator.ts` `checkPredicate` / `checkConditional`: "write rejected" (#4649) | error | | field `requiredWhen` | `rule-validator.ts`: refuses the write (ADR-0137 D2) | error | | field `readonlyWhen` | `isReadonlyWhenLocked`: throws `ValidationError` on every fault but the unbound-root carve-out (ADR-0137 D2) | error | | option `visibleWhen` | `evaluateOptionVisibility`: refuses a write that picks the option (ADR-0137 D2, #22402) | error | | hook `condition` | `hook-wrappers.ts`: aborts the operation (#4775) | error | | formula field `expression` | reads null | warning (text true) | | flow start / edge / `decision` condition | the run fails; the write that fired it lands (`run-fails`) | warning, left out: not a write gate. Open question in the report | | field `visibleWhen` / `settledWhen`, `conditionalFormatting`, field group `visibleWhen`, action `visible` / `disabled` / `pressed`, app landing `visible` | renderer shows, hides or styles; never refuses a write | warning | | sharing rule `condition`; RLS / permission predicates | compiled to a query filter (`compileCelToFilter` / `isSupportedRlsExpression`), not CEL per row | not judged by this verdict | The evaluator's fail-closed behaviour is untouched. **The conversion the message names, measured on the evaluator** (`ExpressionEngine.evaluate`, values `"abc"` / `"7"` / null): `record.name > 5` → FAULT / true / FAULT; `size(record.name) > 5` → false / false / FAULT; `double(record.name) > 5` → FAULT / true / FAULT (validates clean, faults on non-numeric text, so the message says so); `int(...)` same as `double`. ## Tests - `packages/lint/src/validate-expressions.test.ts`: the old pin "warns on a validation predicate ordering a text field against a number" is the behaviour this card retires; it is replaced by five pins: the validation `condition` refusal (path `objects[0].validations[0].condition`, message), the formula warning, one refusal per other fail-closed site (`when`, nested `then`, `requiredWhen`, `readonlyWhen` on a boolean, option `visibleWhen`, hook `condition`), the warning kept on field `visibleWhen` and action `visible`, and the nullable twin unchanged. - `packages/formula/src/validate.test.ts`: `faultOutcome` → error with `code`/`params`, boolean arm, and the two conversions validate clean. - `packages/mcp/src/mcp-validate-expression.test.ts`: `site: 'validation'` → error. - `packages/objectql/src/publish-type-mismatch-fail-closed.test.ts` (new): the real protocol `saveMetaItem` (draft) + `publishPackageDrafts` over a real ObjectQL: refused with `expression-invalid` / `error` at `…validations[0].condition`; formula publishes with the warning advisory; the nullable twin still refused. <details><summary>Package suites (at d5b798c, under os-verify-lock; the final commit 1dfad41 changes only the new objectql test's driver double)</summary> ``` formula test Test Files 64 passed (64) Tests 1629 passed (1629) lint test Test Files 192 passed (192) Tests 7949 passed (7949) mcp test Test Files 55 passed (55) Tests 660 passed (660) spec test Test Files 808 passed (808) Tests 24115 passed | 1 todo (24116) spec test:repo Test Files 89 passed (89) Tests 1524 passed (1524) objectql new file Tests 3 passed (3) typecheck: formula, lint, mcp, spec, objectql — exit 0 ``` </details> <details><summary>Ablation (old severity put back in the fail-closed branch, formula rebuilt, marker proven in dist)</summary> Mutation: `severity: 'error'` → `severity: ['warning', 'ablated-29904'][0] as 'warning'` in the fail-closed arm; `ablation-replace` anchor 1 → 0, `ablation-dist-preflight` marker present in `dist/index.js` and `dist/index.mjs`. ``` lint × refuses a validation predicate ordering a required text field against a number (#29904) AssertionError: expected { …(5) } to match object { severity: 'error', …(1) } × refuses the type mismatch at every other fail-closed site (#29904) Tests 2 failed | 3 passed objectql × refuses `record.name > 5` on a required text field in validations[].condition AssertionError: expected 'published' to be 'refused' formula × errors on the same comparison where the caller declares the site fail-closed (#29904) mcp × errors on a text field ordered against a number at a validation site (#29904) ``` The nullable-twin pins stayed green under the mutation (they are the control). Restore: blob back to HEAD `6fcce907`, `git diff HEAD` empty, formula rebuilt, preflight `--absent` clean on all 6 built files; lint 5/5 and objectql 3/3 green again. </details> <details><summary>Real boot (showcase `pnpm dev -- --fresh`, full build at d5b798c, as admin, writable package com.qa.tmm)</summary> ``` [1] PUT /api/v1/meta/object/tmm_lead?mode=draft (required text `name`, rule `record.name > 5`) → 200 POST /api/v1/packages/com.qa.tmm/publish-drafts → 200 outcome=refused rule=expression-invalid severity=error path=objects.tmm_lead.validations[0].condition message: type mismatch `string > int` — `record.name` holds text but is used with `>` against a number. CEL has no such operation, so wherever the value is not numeric text (an empty value included) the expression faults, and here an expression that cannot be evaluated fails closed: the write (or the operation a hook guards) is refused, not skipped. Compare like with like: … `size(record.name)` for its length, `double(record.name)` for its value (which itself faults on text that is not a number). [1b] same rule as `size(record.name) > 5` → publish 200 outcome=published POST /api/v1/data/tmm_lead {"name":"abc"} → 201 POST /api/v1/data/tmm_lead {"name":"abcdefg"} → 400 VALIDATION_FAILED rule_violation "bad" (the rule now judges) [2] formula `big = record.name > 5` → publish 200 published, advisory expression-invalid severity=warning "… silently evaluates to null …" [4] nullable twin `record.qty > 1000` → publish 200 outcome=refused, expression-invalid error "… declares as nullable …" (unchanged) ``` Before (card, main `589049b0`): the same `[1]` published with a `warning`, and every `POST /data` answered 400 "could not be evaluated … write rejected". The draft save answers 200 for both this rule and the twin, as before (the save door keeps #4463 D1). </details> ## Acceptance notes - **Validation rules of `severity: 'warning'` / `'info'`.** The runtime logs an unevaluable advisory rule instead of refusing; the gate refuses the comparison on every validation rule regardless, exactly as the nullable twin does. Kept for parity with the twin; the message's "refused" clause is exact for `error` rules. - **`readonlyWhen` and option `visibleWhen`** are fail-closed sites the twin does not cover (its `readonlyWhen` exclusion is ruling #4953 clause 3, about the `has()` prescription, which this verdict does not give). Included because their evaluators refuse the write on a fault. - **Serial check.** At 2026-11-03T11:40Z and again at 12:55Z (before opening), none of the open PRs touches `packages/formula/src/validate.ts`, `packages/lint/src/validate-expressions.ts` or `packages/mcp/src/mcp-http-tools.ts` (per-PR file lists read). ## Verification All at the final head **1dfad411f**. - `node scripts/pm/dispatch-gates.mjs --commands`: 106 derived families, each run, all **exit 0**; `--ran` reconciliation: "106 derived, 106 run, 0 NOT-MEASURED, 0 UNRUN". The first round (at d5b798c) had one red, `check:objectql-double-limit` (the new test's copied `find` double ignored `limit`); fixed in 1dfad41, and the whole list was run again. - `pnpm lint` (`eslint . --no-inline-config`): exit 0. - Full dogfood (`pnpm --filter @objectstack/dogfood test`, after a full `turbo run build`, 119/119 tasks): Test Files 410 passed | 3 skipped (413), Tests 3379 passed | 28 skipped (3407); lock VERDICT command-exit 0. - `check-adr-0087-registration --base origin/main`: "registered validation-type-mismatch-refused (new here)". 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WGufzhUVCv2vcJJepWyHQQ <!-- signed: domain:server#6 · os-support-ai · session_01WGufzhUVCv2vcJJepWyHQQ --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7b2bb66 commit 4c2cd29

12 files changed

Lines changed: 519 additions & 26 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/formula': minor
3+
'@objectstack/lint': minor
4+
'@objectstack/mcp': minor
5+
'@objectstack/spec': minor
6+
---
7+
8+
fix(lint)!: publish refuses a validation rule (or another fail-closed predicate) that orders a text or boolean field against a number, instead of warning that it "evaluates to null" and then refusing every write (#29904)
9+
10+
Clause-②: no (narrowing) — the author-time gate (`os build`, `os validate`, the object save door and the package publish) refuses a `type-mismatch` it used to pass with a warning, at the predicate sites that fail closed. Nothing is widened.
11+
Metadata-protocol: narrowed
12+
13+
<!-- adr-0087: registered validation-type-mismatch-refused -->
14+
15+
**BREAKING** accept-set narrowing at the author-time expression gate, served by `os build`, `os validate`, the object save door and `POST /api/v1/packages/:id/publish-drafts`.
16+
17+
- **Before.** `record.name > 5` on a text field, written as a validation rule's `condition`, published with one `expression-invalid` advisory at `severity: warning` saying the expression "silently evaluates to null". A validation rule fails closed, so from then on every create and update whose `name` was not numeric text was refused: `400 VALIDATION_FAILED` "Validation rule 'name_gt' could not be evaluated (runtime: no such overload: dyn<string> > int) — write rejected".
18+
- **Now.** The same rule is refused at publish as `expression-invalid` at `severity: error`, at the rule's `condition` path, with a message that says the write would be refused and how to compare like with like. This is the severity the nullable-comparison verdict (`record.qty > 1000` on a nullable number) already had at the same slot.
19+
- **Which sites.** Those whose runtime refuses the write (or aborts the operation) when the expression faults: a validation rule's `condition` and `when`, nested `then` / `otherwise` rules included; a field's `requiredWhen` and `readonlyWhen`; an option's `visibleWhen` (a write that picks the option); a hook's `condition`. The `validate_expression` MCP tool answers the same for `site: 'validation'`.
20+
- **Unchanged.** A formula field's `expression` keeps the warning: there the fault reads null, which is what the warning says. So do flow conditions (the run fails, the write that fired it lands), a field's display `visibleWhen` / `settledWhen`, field group and action predicates, and `conditionalFormatting`. Sharing-rule conditions compile to a query filter and are not judged by this verdict.
21+
- **`@objectstack/spec`.** The upgrade guide entry `validation-type-mismatch-refused` (a semantic migration: no automatic rewrite, because whether the author meant the length, a number stored as text or another field is a judgement about the rule).
22+
- **`@objectstack/formula`.** `ExprSchemaHint` gains `faultOutcome?: 'fail-closed'`, which the caller sets for a fail-closed site; the `type-mismatch` refusal's `params` carry `outcome: 'fail-closed'` when it is graded an error there.
23+
24+
**FROM → TO.** `condition: 'record.name > 5'` on a text field → `size(record.name) > 5` for its length, `record.score > 5` on a number field for a value, or `double(record.name) > 5` only when every value is numeric text (it faults, and refuses the write, on any other). A boolean: `record.active > 0` → `record.active == true`.

‎content/docs/data-modeling/formulas.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ come at two severities: **errors** fail the build; **warnings** are advisory
242242
| **Qualify field references** — a formula / validation / predicate binds only the `record` namespace, so a bare `amount` resolves to nothing and evaluates to `null`. Write `record.amount`. | `amount > 100` → `record.amount > 100` | error |
243243
| **Field must exist** — a typo'd `record.<field>` is flagged with a did-you-mean. | `record.amont` → `record.amount` | error |
244244
| **Unknown function** — a misspelled or nonexistent stdlib call. | `isBlnk(record.x)` | error |
245-
| **Type soundness** — a text or boolean field used with an arithmetic (`+ - * / %`) or ordering (`< > <= >=`) operator against a number faults at runtime and evaluates to `null`. Store it as a number field, or drop the arithmetic. | `record.title * 2` | warning |
245+
| **Type soundness** — a text or boolean field used with an arithmetic (`+ - * / %`) or ordering (`< > <= >=`) operator against a number faults at runtime unless the text is numeric. In a formula it evaluates to `null`. Where a fault refuses the write instead — a validation rule, a field `requiredWhen` / `readonlyWhen`, an option `visibleWhen`, a hook `condition` — it is an error. Store it as a number field, compare text with text, or convert explicitly (`size(record.name)` for the length; `double(record.name)`, which itself faults on text that is not a number). | `record.title * 2`; `record.name > 5` in a validation rule → `size(record.name) > 5` | warning; error where a fault refuses the write |
246246
| **Date arithmetic** — a `date` / `datetime` field used with `+` / `-` against a number (a `YYYY-MM-DD` value is a string at runtime, so this always faulted to `null`). Use the date helpers instead. | `record.end_date - record.start_date + 1` → `daysBetween(record.start_date, record.end_date) + 1`; `today() + 30` → `daysFromNow(30)`; `record.date + n` → `addDays(record.date, n)` | error (#3306) |
247247
| **Date subtraction** — the runtime holds a `date` / `datetime` field as its text (`YYYY-MM-DD`, or an ISO-8601 instant), so subtracting two of them faults and the formula reads `null`; a timestamp minus one computes a duration, which a field reads as `{}`. Use `daysBetween`. A condition may still compare a duration (`today() - record.start_date > duration("72h")`). | `record.end_date - record.start_date` → `daysBetween(record.start_date, record.end_date)`; `today() - record.start_date` → `daysBetween(record.start_date, today())` | error |
248248

‎packages/formula/src/expression-refusal.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,17 @@ export interface ExpressionRefusalParams {
169169
*/
170170
'date-duration-value': { readonly reference?: string };
171171
/**
172-
* WARNING, never an error: a text or boolean field used with an arithmetic
173-
* or ordering `operator` against a number.
172+
* A text or boolean field used with an arithmetic or ordering `operator`
173+
* against a number. A WARNING (the expression reads null) unless `outcome`
174+
* is `'fail-closed'`: the site refuses the write it guards when the
175+
* expression faults, and the verdict is an ERROR (#29904).
174176
*/
175177
'type-mismatch': {
176178
readonly operands: string;
177179
readonly operator: string;
178180
readonly held: 'text' | 'boolean';
179181
readonly reference?: string;
182+
readonly outcome?: 'fail-closed';
180183
};
181184
/**
182185
* WARNING, never an error: a bare identifier in a flattened condition that

‎packages/formula/src/validate.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,27 @@ describe('validateExpression (ADR-0032)', () => {
764764
expect(r.warnings[0].message).toMatch(/evaluates to null/);
765765
});
766766

767+
// [#29904] The caller declares a fail-closed site; there the fault is a
768+
// refused write, not a null, and the verdict is an error saying so.
769+
it('errors on the same comparison where the caller declares the site fail-closed (#29904)', () => {
770+
const r = validateExpression('predicate', 'record.name > 5', { ...schema, faultOutcome: 'fail-closed' });
771+
expect(r.ok).toBe(false);
772+
expect(r.warnings).toHaveLength(0);
773+
expect(r.errors).toHaveLength(1);
774+
expect(r.errors[0]).toMatchObject({
775+
code: 'type-mismatch',
776+
params: { operands: 'string > int', operator: '>', held: 'text', reference: 'record.name', outcome: 'fail-closed' },
777+
});
778+
expect(r.errors[0].message).not.toMatch(/evaluates to null/);
779+
const bool = validateExpression('predicate', 'record.is_active > 0', { ...schema, faultOutcome: 'fail-closed' });
780+
expect(bool.errors.map((e) => e.code)).toEqual(['type-mismatch']);
781+
// The conversions the message names validate clean in this dialect.
782+
for (const fixed of ['size(record.name) > 5', 'double(record.name) > 5']) {
783+
const ok = validateExpression('predicate', fixed, { ...schema, faultOutcome: 'fail-closed' });
784+
expect([ok.errors, ok.warnings], fixed).toEqual([[], []]);
785+
}
786+
});
787+
767788
it('warns on a text field ordered against a number', () => {
768789
const r = validateExpression('predicate', 'record.title >= 5', schema);
769790
expect(r.ok).toBe(true);

‎packages/formula/src/validate.ts‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,23 @@ export interface ExprSchemaHint {
222222
* {@link fields}; without it nothing is judged.
223223
*/
224224
optionFields?: readonly string[];
225+
/**
226+
* [#29904] What the site's runtime does when this expression FAULTS, as
227+
* declared by the caller that knows the site — the validator cannot tell a
228+
* validation rule's `predicate` from a flow condition's.
229+
*
230+
* `'fail-closed'`: an expression that cannot be evaluated refuses the write
231+
* (or aborts the operation) it guards instead of being skipped — an object
232+
* validation rule, a field `requiredWhen` / `readonlyWhen`, an option
233+
* `visibleWhen` on the write path, a hook `condition`. There a
234+
* `type-mismatch` (text or a boolean ordered / arithmetic'd against a
235+
* number) is an ERROR worded for that outcome, the severity the
236+
* nullable-comparison verdict already has at the same sites.
237+
*
238+
* Absent: a fault reads as null (a formula field), and `type-mismatch` stays
239+
* the advisory warning whose "evaluates to null" text is true there.
240+
*/
241+
faultOutcome?: 'fail-closed';
225242
}
226243

227244
/**
@@ -308,7 +325,9 @@ function toCelFieldTypes(fieldTypes: Readonly<Record<string, string>>): Record<s
308325
* recovers, so it blocks the build.
309326
* - `type-mismatch` (**warning**): a text/boolean field used with an
310327
* arithmetic/ordering operator against a number — nulls unless the text value
311-
* happens to be numeric, so it stays advisory.
328+
* happens to be numeric, so it stays advisory. An **error** where the caller
329+
* declares the site fail-closed (`faultOutcome`, #29904): there the fault is
330+
* not a null, it is a refused write on every value that is not numeric text.
312331
* - `date-text-arithmetic` / `date-duration-value` (**error**, #28944): what
313332
* the Timestamp typing passes, re-checked with a date field declared as the
314333
* text the runtime holds — see {@link dateTextIssue}.
@@ -319,6 +338,7 @@ function typeSoundnessIssue(
319338
fieldTypes: Readonly<Record<string, string>>,
320339
scope: 'record' | 'flattened',
321340
role: 'predicate' | 'value',
341+
faultOutcome: ExprSchemaHint['faultOutcome'],
322342
): { issue: ExprValidationError; severity: 'error' | 'warning' } | null {
323343
const celFieldTypes = toCelFieldTypes(fieldTypes);
324344
const mismatch = firstTypeMismatch(source, celFieldTypes, scope);
@@ -345,6 +365,37 @@ function typeSoundnessIssue(
345365
const heldKind = mismatch.celType === 'bool' ? 'boolean' : 'text';
346366
const held = heldKind === 'boolean' ? 'a boolean' : 'text';
347367
const subject = ref ? `${ref} holds ${held}` : `${held === 'a boolean' ? 'a boolean' : 'a text'} field`;
368+
if (faultOutcome === 'fail-closed') {
369+
// [#29904] Measured on the evaluator: `record.name > 5` faults on `"abc"`
370+
// and on null, and the ADR-0032 §1c rescue answers only numeric text
371+
// (`"7"`). `size()` answers text; `double()` / `int()` validate clean but
372+
// fault on text that is not a number — so the hint says so.
373+
const operand = reference ?? (heldKind === 'boolean' ? 'the field' : 'the text');
374+
const convert = heldKind === 'boolean'
375+
? 'Compare a boolean with `true` / `false`, or use a number field.'
376+
: `Compare like with like: use a number field, compare text with text, or convert ` +
377+
`explicitly — \`size(${operand})\` for its length, \`double(${operand})\` for its value ` +
378+
`(which itself faults on text that is not a number).`;
379+
return {
380+
severity: 'error',
381+
issue: refusal(
382+
'type-mismatch',
383+
{
384+
operands: mismatch.operands,
385+
operator: mismatch.operator,
386+
held: heldKind,
387+
...(reference !== undefined ? { reference } : {}),
388+
outcome: 'fail-closed',
389+
},
390+
source,
391+
`type mismatch \`${mismatch.operands}\` — ${subject} but is used with \`${mismatch.operator}\` ` +
392+
`against a number. CEL has no such operation, so wherever the value is not numeric text ` +
393+
`(an empty value included) the expression faults, and here an expression that cannot be ` +
394+
`evaluated fails closed: the write (or the operation a hook guards) is refused, not skipped. ` +
395+
convert,
396+
),
397+
};
398+
}
348399
return {
349400
severity: 'warning',
350401
issue: refusal(
@@ -1208,10 +1259,11 @@ export function validateExpression(
12081259
// #1928 / #3306 — with per-field types in hand, flag a type-unsound
12091260
// operator use that faults at runtime and silently nulls: a text/boolean
12101261
// field arithmetic'd/ordered against a number (advisory warning — a
1211-
// numeric text value can succeed), or date-field arithmetic (hard error —
1262+
// numeric text value can succeed — or an error where the site fails
1263+
// closed, #29904), or date-field arithmetic (hard error —
12121264
// always nulls). Only runs when there is no bare-ref error (the typed
12131265
// check needs the canonical `record.<field>` form).
1214-
const r = typeSoundnessIssue(source, schema.fieldTypes, 'record', role);
1266+
const r = typeSoundnessIssue(source, schema.fieldTypes, 'record', role, schema.faultOutcome);
12151267
if (r) (r.severity === 'error' ? errors : warnings).push(r.issue);
12161268
}
12171269
} else if (schema?.fields && schema.fields.length > 0) {
@@ -1246,7 +1298,7 @@ export function validateExpression(
12461298
// date-field arithmetic (error). Flow variables stay `dyn` (never flagged);
12471299
// equality/ordering of a date is runtime-safe (never flagged).
12481300
if (schema.fieldTypes) {
1249-
const r = typeSoundnessIssue(source, schema.fieldTypes, 'flattened', role);
1301+
const r = typeSoundnessIssue(source, schema.fieldTypes, 'flattened', role, schema.faultOutcome);
12501302
if (r) (r.severity === 'error' ? errors : warnings).push(r.issue);
12511303
}
12521304
}

‎packages/lint/src/validate-expressions.test.ts‎

Lines changed: 96 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -872,17 +872,108 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
872872
expect(issues).toHaveLength(0);
873873
});
874874

875-
it('warns on a validation predicate ordering a text field against a number', () => {
875+
// [#29904] A validation rule fails closed: a predicate that cannot be
876+
// evaluated refuses the write. So a text field ordered against a number
877+
// there is an ERROR — every write whose value is not numeric text would be
878+
// refused — where a formula's fault reads null and keeps the warning. The
879+
// field is `required` so the nullable-comparison verdict stays out of it.
880+
const typeMismatches = (issues: ReturnType<typeof validateStackExpressions>) =>
881+
issues.filter(i => /^type mismatch/.test(i.message));
882+
883+
it('refuses a validation predicate ordering a required text field against a number (#29904)', () => {
876884
const issues = validateStackExpressions({
877885
objects: [{
878886
name: 'crm_lead',
879-
fields: { title: { type: 'text' } },
887+
fields: { title: { type: 'text', required: true } },
880888
validations: [{ name: 'r', condition: 'record.title > 5' }],
881889
}],
882890
});
883-
const w = issues.filter(i => i.severity === 'warning');
884-
expect(w).toHaveLength(1);
885-
expect(w[0].message).toMatch(/record\.title/);
891+
expect(issues).toHaveLength(1);
892+
expect(issues[0]).toMatchObject({ severity: 'error', path: 'objects[0].validations[0].condition' });
893+
expect(issues[0].message).toMatch(/^type mismatch `string > int` — `record\.title` holds text/);
894+
expect(issues[0].message).toMatch(/the write \(or the operation a hook guards\) is refused/);
895+
expect(issues[0].message).toMatch(/`size\(record\.title\)`/);
896+
expect(issues[0].message).not.toMatch(/evaluates to null/);
897+
});
898+
899+
it('keeps the warning for the same comparison in a formula field, where the fault reads null (#29904)', () => {
900+
const issues = validateStackExpressions({
901+
objects: [{
902+
name: 'crm_lead',
903+
fields: {
904+
title: { type: 'text', required: true },
905+
big: { type: 'formula', expression: 'record.title > 5' },
906+
},
907+
}],
908+
});
909+
expect(issues).toHaveLength(1);
910+
expect(issues[0].severity).toBe('warning');
911+
expect(issues[0].message).toMatch(/silently evaluates to null/);
912+
});
913+
914+
it('refuses the type mismatch at every other fail-closed site (#29904)', () => {
915+
const issues = validateStackExpressions({
916+
objects: [{
917+
name: 'crm_lead',
918+
fields: {
919+
title: { type: 'text', required: true },
920+
active: { type: 'boolean', required: true },
921+
due: { type: 'text', requiredWhen: 'record.title > 5', readonlyWhen: 'record.active > 0' },
922+
tier: {
923+
type: 'select',
924+
options: [{ value: 'gold', label: 'Gold', visibleWhen: 'record.title >= 10' }],
925+
},
926+
},
927+
validations: [
928+
{ name: 'w', type: 'script', condition: 'record.title == "x"', when: 'record.title < 3', message: 'm' },
929+
{
930+
name: 'c',
931+
type: 'conditional',
932+
when: 'record.title == "y"',
933+
then: { name: 'c_then', type: 'script', condition: 'record.title * 2 > 1', message: 'm' },
934+
message: 'm',
935+
},
936+
],
937+
}],
938+
hooks: [{ name: 'h', object: 'crm_lead', events: ['beforeInsert'], condition: 'record.title > 1' }],
939+
});
940+
const found = typeMismatches(issues);
941+
expect(found.map(i => [i.where, i.severity])).toEqual([
942+
["object 'crm_lead' · validation 'w' when", 'error'],
943+
["object 'crm_lead' · validation rule 'c' then → 'c_then'", 'error'],
944+
["object 'crm_lead' · field 'due' requiredWhen", 'error'],
945+
["object 'crm_lead' · field 'due' readonlyWhen", 'error'],
946+
["object 'crm_lead' · field 'tier' option 'gold' visibleWhen", 'error'],
947+
["hook 'h' (crm_lead) condition", 'error'],
948+
]);
949+
expect(found.find(i => /readonlyWhen/.test(i.where))!.message).toMatch(/holds a boolean/);
950+
});
951+
952+
it('keeps the warning where a fault does not refuse a write: field visibleWhen, action visible (#29904)', () => {
953+
const issues = validateStackExpressions({
954+
objects: [{
955+
name: 'crm_lead',
956+
fields: { title: { type: 'text', required: true, visibleWhen: 'record.title > 5' } },
957+
actions: [{ name: 'a', label: 'A', type: 'script', target: 'noop', visible: 'record.title > 5' }],
958+
}],
959+
});
960+
const found = typeMismatches(issues);
961+
expect(found.length).toBeGreaterThan(0);
962+
for (const i of found) expect(i.severity).toBe('warning');
963+
});
964+
965+
it('leaves the nullable-comparison verdict as it was beside the type mismatch (#29904)', () => {
966+
const issues = validateStackExpressions({
967+
objects: [{
968+
name: 'crm_lead',
969+
fields: { qty: { type: 'number' } },
970+
validations: [{ name: 'big', condition: 'record.qty > 1000' }],
971+
}],
972+
});
973+
expect(issues).toHaveLength(1);
974+
expect(issues[0].severity).toBe('error');
975+
expect(issues[0].message).toMatch(/declares as nullable/);
976+
expect(typeMismatches(issues)).toEqual([]);
886977
});
887978

888979
it('warns on a flattened flow condition doing arithmetic on a bare text field', () => {

0 commit comments

Comments
 (0)