diff --git a/.changeset/30494-fail-open-date-sentences.md b/.changeset/30494-fail-open-date-sentences.md new file mode 100644 index 00000000000..58a1509df5e --- /dev/null +++ b/.changeset/30494-fail-open-date-sentences.md @@ -0,0 +1,14 @@ +--- +'@objectstack/formula': patch +--- + +fix(formula): an advisory validation rule's date verdict says the rule is skipped and the write proceeds + +Clause-②: yes (widening) +Metadata-protocol: none + +- **`@objectstack/formula`: two codes join `ExpressionRefusalCode`** (`EXPRESSION_REFUSAL_CODES`), with the params of the code each splits from: `date-arithmetic-fail-open` and `date-text-arithmetic-fail-open`. +- On an advisory (`severity: 'warning'` / `'info'`) validation rule (`faultOutcome: 'fail-open'`), date arithmetic used to be told "this faults at runtime, so the field silently evaluates to null". There is no field there, and nothing reads null: the rule is skipped and logged, and the write proceeds. The verdict now closes on the sentence `type-mismatch-fail-open` already uses for that outcome. This replaces the earlier note that an advisory rule keeps `date-arithmetic` / `date-text-arithmetic`. +- A formula field keeps `date-arithmetic` / `date-text-arithmetic` and the null sentence. A consumer keyed to the old codes on an advisory rule re-keys to the new ones. +- The grade is unchanged: the verdict is an error, as before. What is refused, warned or passed is unchanged. +- `@objectstack/lint` and `@objectstack/mcp` `validate_expression` already pass `faultOutcome: 'fail-open'` for an advisory rule, so both serve the new sentence with no change of their own. diff --git a/packages/formula/src/expression-refusal-codes.test.ts b/packages/formula/src/expression-refusal-codes.test.ts index 62e15c1b98c..f8aae2e37a4 100644 --- a/packages/formula/src/expression-refusal-codes.test.ts +++ b/packages/formula/src/expression-refusal-codes.test.ts @@ -313,6 +313,26 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin, ...Pin 'Use `daysBetween(a, b)` for the span in whole days, and `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)` to shift a date.', }, ], + // [#30494] An advisory (fail-open) rule: the fault skips the rule and the + // write proceeds, nothing reads null. + 'date-arithmetic-fail-open': [ + { + produce: () => + onlyError('predicate', 'record.close_date + 1 > 3', { + scope: 'record', + fieldTypes: { close_date: 'date' }, + faultOutcome: 'fail-open', + }), + params: { operands: 'google.protobuf.Timestamp + int', reference: 'record.close_date' }, + message: + 'date arithmetic `google.protobuf.Timestamp + int` — `record.close_date` is a date, and CEL can\'t do arithmetic ' + + 'on dates: ' + + 'this faults at runtime, and here the rule is advisory, so a rule that cannot be evaluated is skipped, not refused: ' + + 'the write proceeds, the rule is never applied (its advisory never fires), and the only trace is a log line saying ' + + 'it could not be evaluated. ' + + 'Use `daysBetween(a, b)` for the span in whole days, and `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)` to shift a date.', + }, + ], // [#30016] A flattened (flow) condition: the fault fails the run, it never // reads null — `run-fails` where the run is proven to hold the field. 'date-arithmetic-run-fails': [ @@ -377,6 +397,24 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin, ...Pin 'Use `daysBetween(a, b)` for the span in whole days, and `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)` to shift a date.', }, ], + 'date-text-arithmetic-fail-open': [ + { + produce: () => + onlyError('predicate', 'record.end_date - record.start_date > duration("24h")', { + scope: 'record', + fieldTypes: { start_date: 'date', end_date: 'date' }, + faultOutcome: 'fail-open', + }), + params: { operands: 'DateText - DateText', reference: 'record.start_date' }, + message: + 'date arithmetic `DateText - DateText` — `record.start_date` is a date field, and the runtime holds a date field ' + + 'as its text (`YYYY-MM-DD`, or an ISO-8601 instant for a date-time), which CEL can\'t do arithmetic on: ' + + 'this faults at runtime, and here the rule is advisory, so a rule that cannot be evaluated is skipped, not refused: ' + + 'the write proceeds, the rule is never applied (its advisory never fires), and the only trace is a log line saying ' + + 'it could not be evaluated. ' + + 'Use `daysBetween(a, b)` for the span in whole days, and `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)` to shift a date.', + }, + ], 'date-text-arithmetic-run-fails': [ { produce: () => @@ -752,6 +790,15 @@ const SWEEP_SCHEMAS: readonly (ExprSchemaHint | undefined)[] = [ fieldTypes: { close_date: 'date' }, faultOutcome: 'fail-closed', }, + // [#30494] The same date schema on an advisory rule reaches the date + // verdicts' `-fail-open` codes. + { + scope: 'record', + objectName: 'deal', + fields: ['close_date'], + fieldTypes: { close_date: 'date' }, + faultOutcome: 'fail-open', + }, { scope: 'flattened', objectName: 'deal', diff --git a/packages/formula/src/expression-refusal.ts b/packages/formula/src/expression-refusal.ts index 243a4132b94..f69d8c0cc3b 100644 --- a/packages/formula/src/expression-refusal.ts +++ b/packages/formula/src/expression-refusal.ts @@ -167,7 +167,8 @@ export interface ExpressionRefusalParams { * operand types; `reference` is the field as written, when it is known. A * flattened (flow) condition is `date-arithmetic-run-fails` / * `date-arithmetic-run-may-fail`; a fail-closed site is - * `date-arithmetic-fail-closed`. An advisory rule's stays this code. + * `date-arithmetic-fail-closed`, and an advisory rule's is + * `date-arithmetic-fail-open`. */ 'date-arithmetic': { readonly operands: string; readonly reference?: string }; /** @@ -176,7 +177,8 @@ export interface ExpressionRefusalParams { * that text. `operands` are the CEL operand types (the text is `DateText`); * `reference` is the field as written, when it is known. A flattened (flow) * condition is `date-text-arithmetic-run-fails` / `-run-may-fail`; a - * fail-closed site is `date-text-arithmetic-fail-closed`. + * fail-closed site is `date-text-arithmetic-fail-closed`, an advisory rule's + * `date-text-arithmetic-fail-open`. */ 'date-text-arithmetic': { readonly operands: string; readonly reference?: string }; /** @@ -204,6 +206,14 @@ export interface ExpressionRefusalParams { 'date-arithmetic-fail-closed': { readonly operands: string; readonly reference?: string }; /** [#30250] ERROR: `date-text-arithmetic`, at a `date-arithmetic-fail-closed` site. */ 'date-text-arithmetic-fail-closed': { readonly operands: string; readonly reference?: string }; + /** + * [#30494] ERROR: `date-arithmetic` in an advisory (`warning` / `info`) + * validation rule, a site that FAILS OPEN — the fault skips the rule and the + * write proceeds (the `type-mismatch-fail-open` outcome). It never reads null. + */ + 'date-arithmetic-fail-open': { readonly operands: string; readonly reference?: string }; + /** [#30494] ERROR: `date-text-arithmetic`, at a `date-arithmetic-fail-open` site. */ + 'date-text-arithmetic-fail-open': { readonly operands: string; readonly reference?: string }; /** * A formula that computes a `google.protobuf.Duration` * (`today() - record.start_date`), which a field reads as `{}`. `reference` @@ -317,6 +327,8 @@ const CODE_TABLE = { 'date-text-arithmetic-run-may-fail': true, 'date-arithmetic-fail-closed': true, 'date-text-arithmetic-fail-closed': true, + 'date-arithmetic-fail-open': true, + 'date-text-arithmetic-fail-open': true, 'date-duration-value': true, 'type-mismatch': true, 'type-mismatch-fail-closed': true, diff --git a/packages/formula/src/validate.test.ts b/packages/formula/src/validate.test.ts index 1f263290581..114780cbefe 100644 --- a/packages/formula/src/validate.test.ts +++ b/packages/formula/src/validate.test.ts @@ -1139,14 +1139,45 @@ describe('validateExpression (ADR-0032)', () => { ); expect(r.errors[0].message, source).not.toMatch(/evaluates to null/); } - // Controls: a formula field (no outcome) keeps the null sentence; so does - // an advisory rule (`fail-open`), which #30250 does not cover. - for (const [role, source, hint, code] of [ - ['value', 'record.start_date + 1', schema, 'date-arithmetic'], - ['value', 'record.end_date - record.start_date', schema, 'date-text-arithmetic'], - ['predicate', 'record.start_date + 1 > 3', { ...schema, faultOutcome: 'fail-open' }, 'date-arithmetic'], + // Controls: a formula field (no outcome) keeps the null sentence. An + // advisory rule (`fail-open`) is #30494's, below. + for (const [role, source, code] of [ + ['value', 'record.start_date + 1', 'date-arithmetic'], + ['value', 'record.end_date - record.start_date', 'date-text-arithmetic'], ] as const) { - const r = validateExpression(role, source, hint); + const r = validateExpression(role, source, schema); + expect(r.errors.map((e) => e.code), source).toEqual([code]); + expect(r.errors[0].message, source).toContain('this faults at runtime, so the field silently evaluates to null'); + } + }); + + // [#30494] On an advisory (`warning` / `info`) validation rule a fault + // skips the rule and the write proceeds: nothing reads null. The date + // verdicts there close on the `type-mismatch-fail-open` clause, under their + // own codes. The grade stays an error. + it('on an advisory rule says the rule is skipped and the write proceeds, not that the field reads null (#30494)', () => { + const open = { ...schema, faultOutcome: 'fail-open' } as const; + for (const [source, code] of [ + ['record.start_date + 1 > 3', 'date-arithmetic-fail-open'], + ['record.end_date - record.start_date > duration("24h")', 'date-text-arithmetic-fail-open'], + ] as const) { + const r = validateExpression('predicate', source, open); + expect(r.warnings, source).toHaveLength(0); + expect(r.errors.map((e) => e.code), source).toEqual([code]); + expect(r.errors[0].message, source).toContain( + 'this faults at runtime, and here the rule is advisory, so a rule that cannot be evaluated is skipped, not ' + + 'refused: the write proceeds, the rule is never applied (its advisory never fires), and the only trace is a ' + + 'log line saying it could not be evaluated.', + ); + expect(r.errors[0].message, source).not.toMatch(/evaluates to null/); + } + // Control: the same predicates on a formula-field site (no outcome) keep + // their codes and the null sentence. + for (const [source, code] of [ + ['record.start_date + 1 > 3', 'date-arithmetic'], + ['record.end_date - record.start_date > duration("24h")', 'date-text-arithmetic'], + ] as const) { + const r = validateExpression('predicate', source, schema); expect(r.errors.map((e) => e.code), source).toEqual([code]); expect(r.errors[0].message, source).toContain('this faults at runtime, so the field silently evaluates to null'); } diff --git a/packages/formula/src/validate.ts b/packages/formula/src/validate.ts index 4e642385fe4..8931039644a 100644 --- a/packages/formula/src/validate.ts +++ b/packages/formula/src/validate.ts @@ -440,9 +440,10 @@ const DATE_FAIL_CLOSED_CLAUSE = * (`date − date + 1`, `today() + 30`) — always faults at runtime and never * recovers, so it blocks the build. A formula reads that fault as null; a * flattened (flow) condition's fault fails the run, under the - * `date-arithmetic-run-fails` / `-run-may-fail` codes (#30016), and a + * `date-arithmetic-run-fails` / `-run-may-fail` codes (#30016), a * fail-closed site's refuses the write, under `date-arithmetic-fail-closed` - * (#30250, {@link dateArm}). + * (#30250), and an advisory rule's skips the rule, under + * `date-arithmetic-fail-open` (#30494, {@link dateArm}). * - `type-mismatch` (**warning**): a text/boolean field used with an * arithmetic/ordering operator against a number — nulls unless the text value * happens to be numeric, so it stays advisory. An **error** where the caller @@ -611,25 +612,32 @@ function typeSoundnessIssue( * where it holds the date: `run-may-fail`. A `record`-scoped site that fails * closed (`faultOutcome: 'fail-closed'`: an `error` validation rule, a field * write rule, an option `visibleWhen`, a hook `condition`) refuses the write - * on the fault: `fail-closed` (#30250). An advisory rule (`fail-open`) keeps - * the null sentence. The grade stays an error everywhere. + * on the fault: `fail-closed` (#30250). An advisory rule (`faultOutcome: + * 'fail-open'`) is skipped on the fault and the write proceeds: `fail-open`, + * the `type-mismatch-fail-open` clause (#30494). The grade stays an error + * everywhere. */ -type DateArm = 'run-fails' | 'run-may-fail' | 'fail-closed'; +type DateArm = 'run-fails' | 'run-may-fail' | 'fail-closed' | 'fail-open'; function dateArm( scope: 'record' | 'flattened', faultOutcome: ExprSchemaHint['faultOutcome'], bareNamesUnproven: boolean | undefined, ): DateArm | null { - if (scope !== 'flattened') return faultOutcome === 'fail-closed' ? 'fail-closed' : null; + if (scope !== 'flattened') { + return faultOutcome === 'fail-closed' || faultOutcome === 'fail-open' ? faultOutcome : null; + } return faultOutcome === 'run-fails' && !bareNamesUnproven ? 'run-fails' : 'run-may-fail'; } /** - * [#30016, #30250] The fault clause of a date verdict whose fault does not read - * null; `held` names what a flow condition's field holds at run time. + * [#30016, #30250, #30494] The fault clause of a date verdict whose fault does + * not read null; `held` names what a flow condition's field holds at run time. + * An advisory rule's is the type mismatch's own `fail-open` clause, word for + * word: one outcome, one sentence. */ function dateArmFault(arm: DateArm, ref: string | null, held: 'date' | 'text'): string { if (arm === 'fail-closed') return `this faults at runtime, and here ${DATE_FAIL_CLOSED_CLAUSE}`; + if (arm === 'fail-open') return `this faults at runtime, and here ${TYPE_MISMATCH_FAULT_ARMS['fail-open'].clause}`; const fails = 'a flow condition that cannot be evaluated fails the run: the flow stops at this condition and the ' + 'work the flow exists to do is not done (it does not read null).'; @@ -645,8 +653,9 @@ function dateArmFault(arm: DateArm, ref: string | null, held: 'date' | 'text'): * - `date-text-arithmetic`: arithmetic on that text the evaluator faults on — * `record.end_date - record.start_date` (either field kind), `record.d - * duration('24h')` — so the expression reads null (in a flow condition the - * run fails instead, and on a fail-closed site the write is refused: - * `flowArm`, #30016, #30250). Any role. + * run fails instead, on a fail-closed site the write is refused, and on an + * advisory rule the rule is skipped: `flowArm`, #30016, #30250, #30494). + * Any role. * - `date-duration-value`: a formula (`value` role) that computes a * `google.protobuf.Duration` — `today() - record.start_date` — which a field * serialises as `{}`. A predicate that COMPARES a duration diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 1c7255f3108..bc84cfa2feb 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -1382,6 +1382,39 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(formula[0]!.message).toContain('this faults at runtime, so the field silently evaluates to null'); }); + // [#30494] An advisory (`warning` / `info`) validation rule fails open: a + // fault skips the rule and the write proceeds, so its date arithmetic says + // so, still an error. Control: a formula field keeps the null sentence. + it('says an advisory validation rule\'s date arithmetic skips the rule; a formula keeps the null sentence (#30494)', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'crm_opportunity', + fields: { + close_date: { type: 'date' }, + end_date: { type: 'date' }, + next_day: { type: 'formula', expression: 'record.close_date + 1' }, + }, + validations: [ + { name: 'late', condition: 'record.close_date + 1 > 3', severity: 'warning' }, + { name: 'long', condition: 'record.end_date - record.close_date > duration("24h")', severity: 'info' }, + ], + }], + }); + const found = issues.filter(i => /^date arithmetic/.test(i.message)); + expect(found.map(i => i.severity)).toEqual(['error', 'error', 'error']); + const rules = found.filter(i => /validation/.test(i.where)); + expect(rules).toHaveLength(2); + for (const i of rules) { + expect(i.message, i.where).toContain( + 'the rule is advisory, so a rule that cannot be evaluated is skipped, not refused: the write proceeds', + ); + expect(i.message, i.where).not.toMatch(/evaluates to null|is refused, not skipped/); + } + const formula = found.filter(i => !/validation/.test(i.where)); + expect(formula).toHaveLength(1); + expect(formula[0]!.message).toContain('this faults at runtime, so the field silently evaluates to null'); + }); + // [#29959] `seedRunVariables` seeds the flow's declared variables before it // flattens the record, and the flattening skips a name already bound: a // bare `title` here reads the NUMBER variable and `title > 5` runs. Only