diff --git a/.changeset/30250-fail-closed-date-and-sweep-run-fails-sentences.md b/.changeset/30250-fail-closed-date-and-sweep-run-fails-sentences.md new file mode 100644 index 00000000000..b20bff02cd9 --- /dev/null +++ b/.changeset/30250-fail-closed-date-and-sweep-run-fails-sentences.md @@ -0,0 +1,17 @@ +--- +'@objectstack/formula': patch +'@objectstack/lint': patch +'@objectstack/mcp': patch +--- + +fix(formula,lint,mcp): a fail-closed date verdict says the write is refused, and a time-relative sweep's run-fails verdict names no firing write + +Clause-②: yes (widening) +Metadata-protocol: none + +- **`@objectstack/formula`: three codes join `ExpressionRefusalCode`** (`EXPRESSION_REFUSAL_CODES`), with the params of the code they split from: + - `date-arithmetic-fail-closed` and `date-text-arithmetic-fail-closed`. On a site that fails closed (`faultOutcome: 'fail-closed'`: an `error` validation rule, a field `requiredWhen` / `readonlyWhen`, an option `visibleWhen`, a hook `condition`), date arithmetic used to be told "this faults at runtime, so the field silently evaluates to null". There the fault refuses the write instead, and the verdict now says so. A formula field and an advisory (`warning` / `info`) validation rule keep `date-arithmetic` / `date-text-arithmetic` and their sentence. A consumer keyed to the old codes on a fail-closed site re-keys to the new ones. + - `type-mismatch-sweep-run-fails`. A time-relative sweep of the typed object is graded `run-fails` like a record-triggered flow, but no write fires its run: the sweep starts one run per row it finds. Its condition's text-vs-number verdict used to say "the write that fired it still lands". Served where the new optional `ExprSchemaHint.timeRelativeSweep` is set beside `faultOutcome: 'run-fails'`, it now says the run for that row fails and nothing else happens. A record-triggered flow keeps `type-mismatch-run-fails` and its sentence. +- The grade is unchanged at every site. What is refused, warned or passed is unchanged. +- **`@objectstack/lint`**: the flow walk passes `timeRelativeSweep` on a `time_relative` flow. The nullable-comparison verdict, which also says "still lands", runs on record-triggered flows only, so it is unchanged. +- **`@objectstack/mcp`**: `validate_expression` with `site: 'flow_condition'` and `triggerKind: 'time_relative'` passes the same key, so its answer stays the gate's. diff --git a/content/docs/data-modeling/formulas.mdx b/content/docs/data-modeling/formulas.mdx index 2118d79bb00..6d2a5e83dc4 100644 --- a/content/docs/data-modeling/formulas.mdx +++ b/content/docs/data-modeling/formulas.mdx @@ -242,7 +242,7 @@ come at two severities: **errors** fail the build; **warnings** are advisory | **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 | | **Field must exist** — a typo'd `record.` is flagged with a did-you-mean. | `record.amont` → `record.amount` | error | | **Unknown function** — a misspelled or nonexistent stdlib call. | `isBlnk(record.x)` | error | -| **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 does something else it is an error, worded for what happens: a validation rule, a field `requiredWhen` / `readonlyWhen`, an option `visibleWhen` or a hook `condition` refuses the write; a record-triggered flow's start, edge or `decision` branch condition fails the run (the write that fired it still lands), whether it reads the field bare or as `record.`, and so does a time-relative flow's that sweeps the same object; a bare name that a declared flow variable or a `screen` field holds reads that value, so it is not judged as the field, and a bare name after a node that can bind any name (`script`, `connector_action`, `wait`, `subflow`, `map`, a screen with no fields) gets the warning instead, while its `record.` read is still refused; an advisory (`severity: 'warning'` / `'info'`) validation rule is skipped, so it never gives its advisory. 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 in a formula or a display predicate, and in a condition of any other flow, where the warning says the run fails if the condition faults (what such a flow binds under the name depends on how it is started); error at the sites named | +| **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 does something else it is an error, worded for what happens: a validation rule, a field `requiredWhen` / `readonlyWhen`, an option `visibleWhen` or a hook `condition` refuses the write; a record-triggered flow's start, edge or `decision` branch condition fails the run (the write that fired it still lands), whether it reads the field bare or as `record.`, and so does a time-relative flow's that sweeps the same object, where no write fires the run and only the run for that row fails; a bare name that a declared flow variable or a `screen` field holds reads that value, so it is not judged as the field, and a bare name after a node that can bind any name (`script`, `connector_action`, `wait`, `subflow`, `map`, a screen with no fields) gets the warning instead, while its `record.` read is still refused; an advisory (`severity: 'warning'` / `'info'`) validation rule is skipped, so it never gives its advisory. 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 in a formula or a display predicate, and in a condition of any other flow, where the warning says the run fails if the condition faults (what such a flow binds under the name depends on how it is started); error at the sites named | | **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) | | **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 | diff --git a/packages/formula/src/expression-refusal-codes.test.ts b/packages/formula/src/expression-refusal-codes.test.ts index 66b037b91a3..62e15c1b98c 100644 --- a/packages/formula/src/expression-refusal-codes.test.ts +++ b/packages/formula/src/expression-refusal-codes.test.ts @@ -296,6 +296,23 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin, ...Pin 'in whole days, and `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)` to shift a date.', }, ], + // [#30250] A fail-closed site: the fault refuses the write, nothing reads null. + 'date-arithmetic-fail-closed': [ + { + produce: () => + onlyError('predicate', 'record.close_date + 1 > 3', { + scope: 'record', + fieldTypes: { close_date: 'date' }, + faultOutcome: 'fail-closed', + }), + 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 an expression that cannot be evaluated fails closed: the write (or the operation ' + + 'a hook guards) is refused, not skipped (it does not read null). ' + + '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': [ @@ -343,6 +360,23 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin, ...Pin 'and `daysFromNow(n)` / `addDays(d, n)` / `addMonths(d, n)` to shift a date.', }, ], + 'date-text-arithmetic-fail-closed': [ + { + produce: () => + onlyError('predicate', 'record.end_date - record.start_date > duration("24h")', { + scope: 'record', + fieldTypes: { start_date: 'date', end_date: 'date' }, + faultOutcome: 'fail-closed', + }), + 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 an expression that cannot be evaluated fails closed: the write (or the operation ' + + 'a hook guards) is refused, not skipped (it does not read null). ' + + '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: () => @@ -458,6 +492,24 @@ const PINS: { readonly [C in ExpressionRefusalCode]: readonly [Pin, ...Pin 'length, `double(name)` for its value (which itself faults on text that is not a number).', }, ], + // [#30250] The same outcome on a time-relative sweep, which no write fires. + 'type-mismatch-sweep-run-fails': [ + { + produce: () => + onlyError('predicate', 'name > 5', { + scope: 'flattened', fields: ['name'], fieldTypes: { name: 'text' }, faultOutcome: 'run-fails', timeRelativeSweep: true, + }), + params: { operands: 'string > int', operator: '>', held: 'text', reference: 'name' }, + message: + 'type mismatch `string > int` — `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 ' + + 'a flow condition that cannot be evaluated fails the run: the time-relative sweep starts one run per row it finds, ' + + 'the run for this row stops at this condition and the work the flow exists to do is not done for that row, and ' + + 'nothing else happens (the sweep logs the failure and goes on to the next row). ' + + 'Compare like with like: use a number field, compare text with text, or convert explicitly — `size(name)` for its ' + + 'length, `double(name)` for its value (which itself faults on text that is not a number).', + }, + ], 'type-mismatch-fail-open': [ { produce: () => @@ -690,6 +742,24 @@ const SWEEP_SCHEMAS: readonly (ExprSchemaHint | undefined)[] = [ fieldTypes: { name: 'text', active: 'boolean' }, faultOutcome, })), + // [#30250] A fail-closed site with a date field reaches the date verdicts' + // `-fail-closed` codes; a time-relative sweep's `run-fails` condition + // reaches `type-mismatch-sweep-run-fails`. + { + scope: 'record', + objectName: 'deal', + fields: ['close_date'], + fieldTypes: { close_date: 'date' }, + faultOutcome: 'fail-closed', + }, + { + scope: 'flattened', + objectName: 'deal', + fields: ['amount', 'name', 'active'], + fieldTypes: { name: 'text', active: 'boolean' }, + faultOutcome: 'run-fails', + timeRelativeSweep: true, + }, // [#29959] A flattened (flow) condition with no declared outcome reaches // `type-mismatch-run-may-fail`, bare and through `record.`. { scope: 'flattened', objectName: 'deal', fields: ['amount', 'name', 'active'], fieldTypes: { name: 'text', active: 'boolean' } }, diff --git a/packages/formula/src/expression-refusal.ts b/packages/formula/src/expression-refusal.ts index 0c1547ef375..243a4132b94 100644 --- a/packages/formula/src/expression-refusal.ts +++ b/packages/formula/src/expression-refusal.ts @@ -60,7 +60,7 @@ export type ExpressionSourceKind = 'array' | 'object' | 'number' | 'boolean' | ' /** * The params every `type-mismatch*` code interpolates: the CEL operand types, * the `operator`, what the field holds, and the field as written when known. - * The four codes differ in the outcome their sentence states, never in params. + * The codes differ in the outcome their sentence states, never in params. */ interface TypeMismatchParams { readonly operands: string; @@ -166,7 +166,8 @@ export interface ExpressionRefusalParams { * Arithmetic on a date field against a number. `operands` are the CEL * 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`. + * `date-arithmetic-run-may-fail`; a fail-closed site is + * `date-arithmetic-fail-closed`. An advisory rule's stays this code. */ 'date-arithmetic': { readonly operands: string; readonly reference?: string }; /** @@ -174,7 +175,8 @@ export interface ExpressionRefusalParams { * (`record.end_date - record.start_date`), judged with the field declared as * 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`. + * condition is `date-text-arithmetic-run-fails` / `-run-may-fail`; a + * fail-closed site is `date-text-arithmetic-fail-closed`. */ 'date-text-arithmetic': { readonly operands: string; readonly reference?: string }; /** @@ -193,6 +195,15 @@ export interface ExpressionRefusalParams { 'date-text-arithmetic-run-fails': { readonly operands: string; readonly reference?: string }; /** [#30016] ERROR: `date-text-arithmetic`, at a `date-arithmetic-run-may-fail` site. */ 'date-text-arithmetic-run-may-fail': { readonly operands: string; readonly reference?: string }; + /** + * [#30250] ERROR: `date-arithmetic` at a site that FAILS CLOSED (an `error` + * validation rule, a field write rule, an option `visibleWhen`, a hook + * `condition`) — the fault refuses the write (or aborts the operation a + * hook guards). It never reads null. + */ + '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 }; /** * A formula that computes a `google.protobuf.Duration` * (`today() - record.start_date`), which a field reads as `{}`. `reference` @@ -219,6 +230,13 @@ export interface ExpressionRefusalParams { * lands. */ 'type-mismatch-run-fails': TypeMismatchParams; + /** + * [#30250] ERROR: the same mismatch in a condition of a time-relative sweep + * of the typed object (`ExprSchemaHint.timeRelativeSweep`) — the run for + * that row fails at the condition; no write fired it, so nothing else + * happens. + */ + 'type-mismatch-sweep-run-fails': TypeMismatchParams; /** * [#29926] ERROR: the same mismatch in an advisory (`warning` / `info`) * validation rule — the rule is skipped, the write proceeds, and the @@ -297,10 +315,13 @@ const CODE_TABLE = { 'date-arithmetic-run-may-fail': true, 'date-text-arithmetic-run-fails': true, 'date-text-arithmetic-run-may-fail': true, + 'date-arithmetic-fail-closed': true, + 'date-text-arithmetic-fail-closed': true, 'date-duration-value': true, 'type-mismatch': true, 'type-mismatch-fail-closed': true, 'type-mismatch-run-fails': true, + 'type-mismatch-sweep-run-fails': true, 'type-mismatch-fail-open': true, 'type-mismatch-run-may-fail': true, 'field-near-miss': true, diff --git a/packages/formula/src/validate.test.ts b/packages/formula/src/validate.test.ts index add3d12eca8..1f263290581 100644 --- a/packages/formula/src/validate.test.ts +++ b/packages/formula/src/validate.test.ts @@ -806,6 +806,33 @@ describe('validateExpression (ADR-0032)', () => { } }); + // [#30250] A time-relative sweep's run is `run-fails` too (it binds a row + // of the typed object), but no write fires it: the sweep starts one run per + // row it finds. Its sentence names no write, under its own code. + it('on a time-relative sweep says the run for that row fails and names no write (#30250)', () => { + const sweep = { ...schema, scope: 'flattened' as const, faultOutcome: 'run-fails' as const, timeRelativeSweep: true }; + for (const [source, reference] of [['name > 5', 'name'], ['record.name > 5', 'record.name']] as const) { + const r = validateExpression('predicate', source, sweep); + expect(r.warnings, source).toHaveLength(0); + expect(r.errors.map((e) => [e.code, e.params]), source).toEqual([ + ['type-mismatch-sweep-run-fails', { operands: 'string > int', operator: '>', held: 'text', reference }], + ]); + expect(r.errors[0].message, source).toContain( + 'a flow condition that cannot be evaluated fails the run: the time-relative sweep starts one run per row it ' + + 'finds, the run for this row stops at this condition and the work the flow exists to do is not done for that ' + + 'row, and nothing else happens', + ); + expect(r.errors[0].message, source).not.toMatch(/still lands|write that fired|evaluates to null|is refused/); + } + // The key reads only with the `run-fails` outcome: with none declared the + // warning is the same as without it. + const w = validateExpression('predicate', 'name > 5', { ...schema, scope: 'flattened', timeRelativeSweep: true }); + expect(w.warnings.map((e) => e.code)).toEqual(['type-mismatch-run-may-fail']); + // The date verdict's run-fails sentence names no write already: unchanged. + const d = validateExpression('predicate', 'due + 1 > 3', sweep); + expect(d.errors.map((e) => e.code)).toEqual(['date-arithmetic-run-fails']); + }); + // [#29959] A flow condition reads the field bare AND as `record.` // (`seedRunVariables` binds both to the trigger record), so the record // read gets the bare name's verdict: here the run-fails error. @@ -1086,10 +1113,43 @@ describe('validateExpression (ADR-0032)', () => { } // A bare name a flow variable holds is not the field: no verdict. expect(codeOf({ faultOutcome: 'run-fails', flowVariables: ['start_date'] })).toEqual([]); - // Control: a record-scoped validation rule keeps its code and sentence. - const rule = validateExpression('predicate', 'record.start_date + 1 > 3', { ...schema, faultOutcome: 'fail-closed' }); - expect(rule.errors.map((e) => e.code)).toEqual(['date-arithmetic']); - expect(rule.errors[0].message).toMatch(/this faults at runtime, so the field silently evaluates to null/); + // Control: a record-scoped site with no declared outcome (a formula + // field) keeps its code and sentence; a fail-closed rule is #30250's. + const field = validateExpression('value', 'record.start_date + 1', schema); + expect(field.errors.map((e) => e.code)).toEqual(['date-arithmetic']); + expect(field.errors[0].message).toMatch(/this faults at runtime, so the field silently evaluates to null/); + }); + + // [#30250] On a fail-closed site (an `error` validation rule, a field + // `requiredWhen` / `readonlyWhen`, an option `visibleWhen`, a hook + // `condition`) a fault refuses the write: nothing reads null. The date + // verdicts there take the fail-closed sentence under their own codes. + it('on a fail-closed site says the write is refused, not that the field reads null (#30250)', () => { + const closed = { ...schema, faultOutcome: 'fail-closed' } as const; + for (const [source, code] of [ + ['record.start_date + 1 > 3', 'date-arithmetic-fail-closed'], + ['record.end_date - record.start_date > duration("24h")', 'date-text-arithmetic-fail-closed'], + ] as const) { + const r = validateExpression('predicate', source, closed); + 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 an expression that cannot be evaluated fails closed: the write (or the ' + + 'operation a hook guards) is refused, not skipped (it does not read null).', + ); + 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'], + ] as const) { + const r = validateExpression(role, source, hint); + 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 9c19c3c3ddd..4e642385fe4 100644 --- a/packages/formula/src/validate.ts +++ b/packages/formula/src/validate.ts @@ -240,7 +240,9 @@ export interface ExprSchemaHint { * `'run-fails'`: a record-triggered flow's start, edge or `decision` branch condition — the * engine's `evaluateCondition` throws on the fault (ADR-0032 §1c), so the * run fails there, while the record-change trigger isolates the write that - * fired it: the write lands, the flow's work is not done. + * fired it: the write lands, the flow's work is not done. A time-relative + * sweep of the typed object takes it too; no write fires that run, which + * {@link timeRelativeSweep} says (#30250). * * `'fail-open'`: an advisory (`severity: 'warning' | 'info'`) validation * rule — a rule that cannot be evaluated is skipped and logged, the write @@ -279,6 +281,17 @@ export interface ExprSchemaHint { * and keeps the outcome. Absent ⇒ every verdict is exactly as before this key. */ bareNamesUnproven?: boolean; + /** + * [#30250] `'flattened'` scope with {@link faultOutcome} `'run-fails'` only: + * the flow is a time-relative sweep's (`@objectstack/lint`'s flow walk on a + * `time_relative` trigger). Its run fails at a faulting condition as a + * record-triggered run does, but no write fires it — `time-relative-trigger.ts` + * starts one run per row the sweep finds, logs a failed run and goes on to + * the next row — so the `type-mismatch` verdict names no write that "still + * lands": it is `type-mismatch-sweep-run-fails`. Read with no other outcome. + * Absent ⇒ every verdict is exactly as before this key. + */ + timeRelativeSweep?: boolean; } /** @@ -397,6 +410,29 @@ const TYPE_MISMATCH_FAULT_ARMS: { }, }; +/** + * [#30250] The `run-fails` outcome on a time-relative sweep + * ({@link ExprSchemaHint.timeRelativeSweep}): the run fails as a + * record-triggered one does, but no write fired it, so the clause names none. + * Its own code: a code names one message template. + */ +const TYPE_MISMATCH_SWEEP_ARM = { + code: 'type-mismatch-sweep-run-fails', + clause: + 'a flow condition that cannot be evaluated fails the run: the time-relative sweep starts one run per row it ' + + 'finds, the run for this row stops at this condition and the work the flow exists to do is not done for that ' + + 'row, and nothing else happens (the sweep logs the failure and goes on to the next row).', +} as const; + +/** + * [#30250] The `fail-closed` clause a date verdict closes on: the type + * mismatch's, which says the write is refused, plus what the old sentence + * got wrong. + */ +const DATE_FAIL_CLOSED_CLAUSE = + 'an expression that cannot be evaluated fails closed: the write (or the operation a hook guards) is refused, ' + + 'not skipped (it does not read null).'; + /** * #1928 / #3306 — a type-soundness verdict for an expression, or `null` when * sound. Two categories, with different severities: @@ -404,8 +440,9 @@ const TYPE_MISMATCH_FAULT_ARMS: { * (`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, - * {@link dateFlowArm}). + * `date-arithmetic-run-fails` / `-run-may-fail` codes (#30016), and a + * fail-closed site's refuses the write, under `date-arithmetic-fail-closed` + * (#30250, {@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 @@ -420,7 +457,8 @@ const TYPE_MISMATCH_FAULT_ARMS: { * - `date-text-arithmetic` / `date-duration-value` (**error**, #28944): what * the Timestamp typing passes, re-checked with a date field declared as the * text the runtime holds — see {@link dateTextIssue}; in a flow condition - * the fault takes the same two flow codes as `date-arith`. + * and on a fail-closed site the fault takes the same outcome codes as + * `date-arith`. * `scope` selects `record.` vs bare field binding, shaping the message. */ function typeSoundnessIssue( @@ -431,6 +469,7 @@ function typeSoundnessIssue( faultOutcome: ExprSchemaHint['faultOutcome'], flowVariables?: readonly string[], bareNamesUnproven?: boolean, + timeRelativeSweep?: boolean, ): { issue: ExprValidationError; severity: 'error' | 'warning' } | null { const celFieldTypes = toCelFieldTypes(fieldTypes); // [#29959] A bare name a flow variable holds is not the field (see @@ -457,7 +496,7 @@ function typeSoundnessIssue( const bareCelTypes = shadowed ? Object.fromEntries(Object.entries(celFieldTypes).filter(([name]) => !shadowed.has(name))) : celFieldTypes; - return dateTextIssue(source, bareCelTypes, scope, role, dateFlowArm(scope, faultOutcome, bareNamesUnproven)); + return dateTextIssue(source, bareCelTypes, scope, role, dateArm(scope, faultOutcome, bareNamesUnproven)); } const reference = mismatch.field ? (mismatch.root !== undefined @@ -472,11 +511,11 @@ function typeSoundnessIssue( const use = `Use \`daysBetween(a, b)\` for the span in whole days, and \`daysFromNow(n)\` / ` + `\`addDays(d, n)\` / \`addMonths(d, n)\` to shift a date.`; - const arm = dateFlowArm(scope, faultOutcome, bareNamesUnproven); + const arm = dateArm(scope, faultOutcome, bareNamesUnproven); return { severity: 'error', issue: arm - ? refusal(`date-arithmetic-${arm}`, params, source, `${head}${dateFlowFault(arm, ref, 'date')} ${use}`) + ? refusal(`date-arithmetic-${arm}`, params, source, `${head}${dateArmFault(arm, ref, 'date')} ${use}`) : refusal( 'date-arithmetic', params, @@ -511,7 +550,9 @@ function typeSoundnessIssue( if (outcome !== undefined) { // [#29926] One code per outcome: a code names ONE message template // (`expression-refusal.ts`), and the three outcomes are three sentences. - const arm = TYPE_MISMATCH_FAULT_ARMS[outcome]; + const arm = outcome === 'run-fails' && scope === 'flattened' && timeRelativeSweep + ? TYPE_MISMATCH_SWEEP_ARM + : TYPE_MISMATCH_FAULT_ARMS[outcome]; return { severity: 'error', issue: refusal(arm.code, params, source, `${faults}, and here ${arm.clause} ${convert}`), @@ -558,28 +599,37 @@ function typeSoundnessIssue( } /** - * [#30016] Which flow sentence a date verdict takes, or `null` off a flow. A - * flattened site is a flow condition, and the engine's `evaluateCondition` - * throws on a fault (ADR-0032 §1c) on every flow: a date verdict there never - * reads null. `run-fails` where the caller declares the run proven to hold the - * field (`faultOutcome: 'run-fails'`) and no node can rebind a bare name + * [#30016, #30250] Which outcome sentence a date verdict takes, or `null` where + * the fault reads null (a formula field, a display predicate). A flattened + * site is a flow condition, and the engine's `evaluateCondition` throws on a + * fault (ADR-0032 §1c) on every flow: a date verdict there never reads null. + * `run-fails` where the caller declares the run proven to hold the field + * (`faultOutcome: 'run-fails'`) and no node can rebind a bare name * (`bareNamesUnproven`) — the date verdicts read bare names only. Anywhere * else in a flow the name holds what the entrance handed the run (an API body * whose value is a number answers `close_date + 1`), so the run fails only - * where it holds the date: `run-may-fail`. The grade stays an error either way. + * 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. */ -type DateFlowArm = 'run-fails' | 'run-may-fail'; -function dateFlowArm( +type DateArm = 'run-fails' | 'run-may-fail' | 'fail-closed'; +function dateArm( scope: 'record' | 'flattened', faultOutcome: ExprSchemaHint['faultOutcome'], bareNamesUnproven: boolean | undefined, -): DateFlowArm | null { - if (scope !== 'flattened') return null; +): DateArm | null { + if (scope !== 'flattened') return faultOutcome === 'fail-closed' ? 'fail-closed' : null; return faultOutcome === 'run-fails' && !bareNamesUnproven ? 'run-fails' : 'run-may-fail'; } -/** [#30016] The fault clause of a flow date verdict; `held` names what the field holds at run time. */ -function dateFlowFault(arm: DateFlowArm, ref: string | null, held: 'date' | 'text'): string { +/** + * [#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. + */ +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}`; 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).'; @@ -595,7 +645,8 @@ function dateFlowFault(arm: DateFlowArm, ref: string | null, held: 'date' | 'tex * - `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: `flowArm`, #30016). Any role. + * run fails instead, and on a fail-closed site the write is refused: + * `flowArm`, #30016, #30250). 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 @@ -608,7 +659,7 @@ function dateTextIssue( celFieldTypes: Readonly>, scope: 'record' | 'flattened', role: 'predicate' | 'value', - flowArm: DateFlowArm | null, + flowArm: DateArm | null, ): { issue: ExprValidationError; severity: 'error' | 'warning' } | null { if (role !== 'value' && !Object.values(celFieldTypes).includes('timestamp')) return null; const verdict = firstDateTextVerdict(source, celFieldTypes, scope); @@ -629,7 +680,7 @@ function dateTextIssue( return { severity: 'error', issue: flowArm - ? refusal(`date-text-arithmetic-${flowArm}`, params, source, `${head}${dateFlowFault(flowArm, ref, 'text')} ${use}`) + ? refusal(`date-text-arithmetic-${flowArm}`, params, source, `${head}${dateArmFault(flowArm, ref, 'text')} ${use}`) : refusal( 'date-text-arithmetic', params, @@ -1480,7 +1531,7 @@ export function validateExpression( // equality/ordering of a date is runtime-safe (never flagged). if (schema.fieldTypes) { const r = typeSoundnessIssue(source, schema.fieldTypes, 'flattened', role, schema.faultOutcome, schema.flowVariables, - schema.bareNamesUnproven); + schema.bareNamesUnproven, schema.timeRelativeSweep); if (r) (r.severity === 'error' ? errors : warnings).push(r.issue); } } diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index d80f37cfd91..1c7255f3108 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -1146,13 +1146,56 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { [`flow '${name}' · edge 'e1' (start→route) condition`, severity], ]); for (const i of found) { + // [#30250] No write fires a sweep's run: its sentence names none. expect(i.message, `${name} ${i.where}`).toContain( - severity === 'error' ? 'the write that fired it still lands' : 'so this is a warning, not a refusal', + severity === 'error' + ? 'the time-relative sweep starts one run per row it finds, the run for this row stops at this condition' + : 'so this is a warning, not a refusal', ); + expect(i.message, `${name} ${i.where}`).not.toMatch(/still lands|write that fired/); } } }); + // [#30250] A time-relative sweep's run is fired by no write. The type + // verdict's run-fails sentence says the run for that row fails and nothing + // else happens; the nullable-comparison twin is not served on a sweep at + // all (it runs on record-triggered flows only). Control: the same flow + // record-triggered keeps the sentence about the write that fired it. + it('names no firing write on a time-relative sweep, and keeps it on a record-triggered flow (#30250)', () => { + const objects = [{ + name: 'crm_opportunity', + fields: { title: { type: 'text', required: true }, budget: { type: 'number' }, close_date: { type: 'date' } }, + }]; + const flowOf = (name: string, startConfig: Record) => ({ + name, + nodes: [ + { id: 'start', type: 'start', config: { objectName: 'crm_opportunity', condition: 'title > 5', ...startConfig } }, + { id: 'end', type: 'end' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end', condition: 'budget > 100' }], + }); + const sweep = validateStackExpressions({ + objects, + flows: [flowOf('tr_sweep', { + schedule: '0 * * * *', timeRelative: { object: 'crm_opportunity', dateField: 'close_date', offsetDays: 3 }, + })], + }); + const sweepType = typeMismatches(sweep); + expect(sweepType.map(i => i.severity)).toEqual(['error']); + expect(sweepType[0]!.message).toContain('and nothing else happens (the sweep logs the failure and goes on to the next row)'); + for (const i of sweep) expect(i.message, i.where).not.toMatch(/still lands|write that fired/); + const recordTriggered = validateStackExpressions({ + objects, flows: [flowOf('rt', { triggerType: 'record-after-update' })], + }); + const rtType = typeMismatches(recordTriggered); + expect(rtType.map(i => i.severity)).toEqual(['error']); + expect(rtType[0]!.message).toContain('the write that fired it still lands'); + // The null-guard twin on the record-triggered edge: its sentence too. + expect(recordTriggered.filter(i => /budget/.test(i.message) && /still lands/.test(i.message)).map(i => i.where)) + .toEqual(["flow 'rt' · edge 'e1' (start→end) condition"]); + }); + // [#30026] Where the sweep is of another object, the author is told why the // verdict is only a warning: once, at the start node's `objectName`, and // only on a flow that was served a warning it explains. @@ -1308,17 +1351,35 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { } }); - it('keeps the null sentence on a record-scoped validation rule\'s date arithmetic (#30016 control)', () => { + // [#30250] An `error` validation rule fails closed: its date arithmetic + // refuses the write, so it says so. Control: a formula field's date + // arithmetic reads null, and keeps the null sentence. + it('says a fail-closed validation rule\'s date arithmetic refuses the write; a formula keeps the null sentence (#30250)', () => { const issues = validateStackExpressions({ objects: [{ name: 'crm_opportunity', - fields: { close_date: { type: 'date' } }, - validations: [{ name: 'late', condition: 'record.close_date + 1 > 3' }], + 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' }, + { name: 'long', condition: 'record.end_date - record.close_date > duration("24h")' }, + ], }], }); const found = issues.filter(i => /^date arithmetic/.test(i.message)); - expect(found.map(i => i.severity)).toEqual(['error']); - expect(found[0]!.message).toContain('this faults at runtime, so the field silently evaluates to null'); + 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('fails closed: the write (or the operation a hook guards) is refused, not skipped'); + expect(i.message, i.where).not.toMatch(/evaluates to null/); + } + 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 diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 3edcb93e9e6..66c75435a1e 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -540,6 +540,12 @@ interface FlowBareNames { readonly variables: readonly string[]; /** The run can bind a name no reader sees: a bare-name mismatch is the warning. */ readonly unproven: boolean; + /** + * [#30250] The flow is a time-relative sweep: its `run-fails` verdict names + * no write that fired the run (`ExprSchemaHint.timeRelativeSweep`). Rides + * here because every flow site that reads the names reads it too. + */ + readonly timeRelativeSweep: boolean; } function buildNullableFieldIndex(objects: AnyRec[]): Map> { @@ -1313,7 +1319,7 @@ function isFlattenedNearMissFor(diagnostic: string, roots: readonly string[]): b * | option `visibleWhen` | `evaluateOptionVisibility`: refuses a write picking it (ADR-0137 D2, #22402) | `fail-closed` | * | hook `condition` | `hook-wrappers.ts`: aborts the operation (#4775) | `fail-closed` | * | flow start / edge condition, `decision` branch — record-triggered flow | `evaluateCondition` throws (ADR-0032 §1c); the trigger isolates the write: the run fails, the write lands. A bare name and its `record.` read are the same total trigger record (#29959) | `run-fails` | - * | the same, time-relative flow sweeping the flow's own object | `evaluateCondition` throws; `time-relative-trigger.ts` launches each run with the swept row of that object as `record` (`{ record: row }`, the stored row a find returns), so the field's type is what the name holds there too (#29959) | `run-fails` | + * | the same, time-relative flow sweeping the flow's own object | `evaluateCondition` throws; `time-relative-trigger.ts` launches each run with the swept row of that object as `record` (`{ record: row }`, the stored row a find returns), so the field's type is what the name holds there too (#29959). No write fires the run: the sweep starts one per row it finds, logs a failed one and goes on to the next row | `run-fails`, with `ExprSchemaHint.timeRelativeSweep` so the sentence names no firing write (#30250) | * | flow condition, any other flow (API, schedule, no trigger, or a sweep of another object than the one the conditions are typed against) | `evaluateCondition` throws there too, but no row of the typed object is bound (see `validate-null-guards.ts`'s ledger): the run holds what its entrance handed it — an API request body, an action's partial record, a parent's record, or nothing — so the field's type is not proven to be what the name holds | — not declared: the `type-mismatch-run-may-fail` WARNING, worded for the run (#29959) | * | a bare name the flow's run binds as a variable (a declared variable, a node id, a `screen` field's `name`) | the run seeds it before the flattening, which skips a bound name, or a screen's resume lands it over the flattened field: it holds the variable, not the field | — not typed as the field at all (`ExprSchemaHint.flowVariables`); `record.` still is (#29959, #30025) | | a bare name in a flow a node of which can bind names no reader sees (`flowCelRootScope` read with no entrance: an open binder — `script`, `connector_action`, `wait`, `subflow`, `map` —, a screen with no input contract) | the run can bind the name to a value no reader sees | — no declared outcome for a bare-name mismatch: the `type-mismatch-run-may-fail` WARNING (`ExprSchemaHint.bareNamesUnproven`); `record.` keeps the row's outcome (#30025) | @@ -1331,7 +1337,8 @@ function isFlattenedNearMissFor(diagnostic: string, roots: readonly string[]): b * never gives its advisory — the message says so instead of a refused write. * The date verdicts (`date-arithmetic`, `date-text-arithmetic`) are an error * at every site; in a flow condition the same outcome picks their sentence: - * `-run-fails` under `run-fails`, `-run-may-fail` anywhere else (#30016). + * `-run-fails` under `run-fails`, `-run-may-fail` anywhere else (#30016); on + * a `fail-closed` site it is `-fail-closed`, the refused write (#30250). */ type FaultSite = | { readonly kind: 'validation rule'; readonly severity: unknown } @@ -2531,6 +2538,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression ...(faultOutcome ? { faultOutcome } : {}), ...(flowBare ? { flowVariables: flowBare.variables } : {}), ...(flowBare?.unproven ? { bareNamesUnproven: true } : {}), + ...(flowBare?.timeRelativeSweep ? { timeRelativeSweep: true } : {}), } : { scope, ...roots }); for (const e of res.errors) { @@ -2878,6 +2886,9 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression const flowBare: FlowBareNames = { variables: [...new Set([...declaredVariables, ...screenFields, ...FLOW_ENGINE_BOUND_NAMES])], unproven: !flowCelRootScope(flow, graphs, objectName, fieldIndex).provable, + // [#30250] No write fires a sweep's run: `time-relative-trigger.ts` starts + // one per row it finds. The validator reads it only under `run-fails`. + timeRelativeSweep: triggerKind === 'time_relative', }; const nullGuardBare = new Set(); if (nullGuardObject && rootScope.provable) { diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index d6592578d35..3417703fae7 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -1202,6 +1202,9 @@ export function registerObjectTools( scope, ...(optionFields ? { optionFields } : {}), ...(faultOutcome ? { faultOutcome } : {}), + // [#30250] A time-relative sweep's run is fired by no write: the + // gate's run-fails sentence there names none, and so does this one. + ...(site === 'flow_condition' && triggerKind === 'time_relative' ? { timeRelativeSweep: true } : {}), } as const; // The gate's envelope verdict first: `validateExpression` treats a // blank source as "not authored" and answers ok, while the save gate