Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/30494-fail-open-date-sentences.md
Original file line number Diff line number Diff line change
@@ -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.
47 changes: 47 additions & 0 deletions packages/formula/src/expression-refusal-codes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,26 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin<C>, ...Pin<C>
'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': [
Expand Down Expand Up @@ -377,6 +397,24 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin<C>, ...Pin<C>
'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: () =>
Expand Down Expand Up @@ -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',
Expand Down
16 changes: 14 additions & 2 deletions packages/formula/src/expression-refusal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
/**
Expand All @@ -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 };
/**
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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,
Expand Down
45 changes: 38 additions & 7 deletions packages/formula/src/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down
29 changes: 19 additions & 10 deletions packages/formula/src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).';
Expand All @@ -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
Expand Down
33 changes: 33 additions & 0 deletions packages/lint/src/validate-expressions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading