fix(lint)!: a flow condition's text-vs-number comparison gets a true verdict on every flow, record.x read and decision branch

.changeset/29959-flow-condition-type-mismatch-every-site.md

@@ -0,0 +1,26 @@
+---
+'@objectstack/formula': minor
+'@objectstack/lint': minor
+'@objectstack/mcp': minor
+'@objectstack/spec': minor
+---
+
+fix(lint)!: every flow condition's text-vs-number comparison gets a verdict that says what the run does — a record-triggered flow's `record.<field>` read and `decision` branch are refused like its bare name, a time-relative sweep of the flow's object is refused like a record-triggered flow, a bare name a flow variable holds is no longer typed as the field, and any other flow's warning says the run fails instead of "evaluates to null" (#29959)
+
+Clause-②: yes (narrowing) — the author-time gate (`os build`, `os validate`, the package publish and the flow publish door) refuses, on a record-triggered flow, a `type-mismatch` in a `decision` branch and in a `record.<field>` / `previous.<field>` read of a start or edge condition, which it passed with no verdict before; refuses the same on a time-relative flow sweeping the object its conditions are typed against, where 18.3.0 served a warning; and a flow condition's `type-mismatch` warning is served under its own code `type-mismatch-run-may-fail` instead of `type-mismatch`. Added beside them (the widening half): the optional `severity` and `triggerKind` inputs of the `validate_expression` MCP tool, the code `type-mismatch-run-may-fail` in `@objectstack/formula`'s `ExpressionRefusalCode`, the optional `flowVariables` key of `@objectstack/formula`'s `ExprSchemaHint`, and a bare flow-condition name that a declared flow variable or node id holds is no longer typed as the field it shadows (no verdict where 18.3.0 served a `type-mismatch` warning).
+Metadata-protocol: narrowed
+Skills-docs: updated here — `content/docs/data-modeling/formulas.mdx` (the type-soundness row names the decision branch, the `record.<field>` read and the warning on other flows) and the MCP skill text in `packages/mcp/src/skill-md.ts` (the two new `validate_expression` inputs); `skills/**` teaches no text-vs-number comparison in a flow condition (grep for `type mismatch`, `evaluates to null`, `validate_expression` and the decision examples), so no skill or eval moves.
+
+<!-- adr-0087: registered flow-record-read-and-decision-type-mismatch-refused -->
+
+**BREAKING** accept-set narrowing at the author-time expression gate, served by `os build`, `os validate` and `POST /api/v1/packages/:id/publish-drafts`.
+
+- **Record-triggered flows — before.** The start and edge conditions refused `name > 5` on a text field (`type-mismatch-run-fails`), but `record.name > 5` in the same slot, and `name > 5` in a `decision` node's `config.conditions[].expression`, published with no verdict at all. Both fail the run the same way: the engine's `evaluateCondition` throws on the fault, the run binds the trigger record both as `record` and flattened to bare names, and a decision branch is evaluated over the same variables as an edge.
+- **Now.** Both are refused as `expression-invalid` at `severity: error`, code `type-mismatch-run-fails`, with the message the bare name already gets: the run fails at the condition and the triggering write still lands. `previous.<field>` is judged the same way. The date verdicts stay on the bare name only, as before.
+- **Time-relative flows.** A sweep launches each run with the swept row as its record, so on a flow whose `timeRelative.object` is the start node's `objectName` (or is omitted) the start, edge and `decision` conditions get the record-triggered verdict above, bare name included: `type-mismatch-run-fails`, where 18.3.0 served a `type-mismatch` warning. A sweep of another object keeps the warning below.
+- **Any other flow** (no `triggerType`, a `schedule` flow, an `api` flow) and an inert node `condition` keep a warning. Its text said the expression "silently evaluates to null", which is false on every flow: a condition that faults fails the run. It is now `type-mismatch-run-may-fail`, worded for the failed run, and a warning because such a flow is handed whatever its entrance carries (an API request body, an action's partial record, a parent flow's record, or nothing), so the field's type is not proven to be what the name holds. A `record.<field>` read and a `decision` branch there get the same warning. The flow registration log line reads the same text.
+- **A bare name a flow variable holds.** The run seeds a flow's declared variables (and each node's outputs, under its id) before it flattens the record, and the flattening skips a name already bound. So in a flow that declares `variables: [{ name: 'title', type: 'number' }]` over a text field `title`, a bare `title > 5` reads the number and runs; it was refused as `type-mismatch-run-fails` on the head that added that error, and warned on 18.3.0. It now gets no type verdict (the `flow-variable-shadows-field` warning still names the shadowing). `record.title > 5` reads the field and stays refused.
+- **`validate_expression` (MCP).** Two optional inputs, so the tool answers what the gate will: `severity` (`error` | `warning` | `info`, site `validation`; default `error`) gives an advisory rule the gate's `type-mismatch-fail-open` instead of the fail-closed verdict, and `triggerKind` (`record_change` | `time_relative` | `schedule` | `api` | `none`, site `flow_condition`; default `record_change`) gives a `schedule`, `api` or `none` flow the gate's warning instead of the run-fails error (`time_relative` answers run-fails, its record being a row of `objectName`). Either input on a site it does not describe is refused as a tool error. A call that passes neither answers as before.
+- **`@objectstack/spec`.** Upgrade guide entry `flow-record-read-and-decision-type-mismatch-refused` (semantic: no mechanical rewrite).
+
+**FROM → TO.** In a record-triggered flow, or a time-relative flow sweeping its own object: `record.name > 5` (or a decision branch `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. A boolean: `record.active > 0` → `record.active == true`. A consumer keyed to the code of a flow condition's warning: `type-mismatch` → `type-mismatch-run-may-fail` (`@objectstack/formula@18.3.0` and `@objectstack/mcp@18.3.0` serve `type-mismatch` there; the record-scoped `type-mismatch`, on a formula field or a display predicate, is unchanged).

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.<field>` 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 or edge condition fails the run (the write that fired it still lands); 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; 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.<field>`, and so does a time-relative flow's that sweeps the same object; a bare name that a declared flow variable holds reads the variable, so it is not judged as the field; 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 |
 

packages/formula/src/cel-engine.ts

@@ -966,6 +966,14 @@ function buildTypedEnv(
   fieldCelTypes: Readonly<Record<string, FieldCelType>>,
   scope: 'record' | 'flattened',
   temporalAsText = false,
+  /**
+   * [#29959] Flattened scope only: the field types to type `record.<field>` /
+   * `previous.<field>` with. Absent, those roots stay `map` (dyn members). It
+   * is a map of its own, not `fieldCelTypes`, because a bare name a flow
+   * variable holds is left out of the bare typing while `record.<name>` still
+   * reads the field.
+   */
+  recordRootTypes?: Readonly<Record<string, FieldCelType>>,
 ): Environment {
   // `temporalAsText` declares a `date`/`datetime` field as the text the runtime
   // holds ({@link DATE_TEXT_CEL_TYPE}) instead of a Timestamp — see
@@ -981,6 +989,18 @@ function buildTypedEnv(
     registerStdLib(env, () => new Date(0));
     registerNullComparisons(env);
     if (temporalAsText) registerDateTextOverloads(env);
+    if (recordRootTypes) {
+      // [#29959] A flow run binds the record it was handed as `record` too
+      // (`seedRunVariables`), beside the flattened bare names, so
+      // `record.<field>` is the same value as the bare `<field>` and is typed
+      // the same: the `record`-scope struct, on `record` and `previous`.
+      const fields: Record<string, string> = {};
+      for (const [name, t] of Object.entries(recordRootTypes)) fields[name] = typeName(t);
+      try { env.registerType('OsRecordScope', { fields }); } catch { /* invalid field name — ignore */ }
+      for (const root of ['record', 'previous']) {
+        try { env.registerVariable(root, 'OsRecordScope'); } catch { /* duplicate — ignore */ }
+      }
+    }
     for (const root of SCOPE_ROOTS) {
       try { env.registerVariable(root, 'map'); } catch { /* duplicate — ignore */ }
     }
@@ -1038,6 +1058,39 @@ function offendingField(
   return null;
 }
 
+/**
+ * [#29959] `fieldCelTypes` without `names` — the bare typing of a flattened
+ * condition whose run binds those names as variables, which win over the
+ * field (see {@link firstTypeMismatch}'s `bareShadowed`).
+ */
+function withoutNames(
+  fieldCelTypes: Readonly<Record<string, FieldCelType>>,
+  names: ReadonlySet<string>,
+): Record<string, FieldCelType> {
+  const out: Record<string, FieldCelType> = {};
+  for (const [name, t] of Object.entries(fieldCelTypes)) if (!names.has(name)) out[name] = t;
+  return out;
+}
+
+/**
+ * [#29959] The `record.<field>` / `previous.<field>` read of a field of
+ * `celType` in a flattened condition, as written — what
+ * {@link firstTypeMismatch}'s record-root pass names. `null` when no such read
+ * is spelled member-wise (an index read, say), so the message names no field.
+ */
+function offendingRecordRead(
+  source: string,
+  fieldCelTypes: Readonly<Record<string, FieldCelType>>,
+  celType: FieldCelType,
+): { field: string; root: 'record' | 'previous' } | null {
+  for (const [name, t] of Object.entries(fieldCelTypes)) {
+    if (t !== celType) continue;
+    const m = new RegExp(`(?<![\\w$.])(record|previous)\\.${name}(?![\\w$])`).exec(source);
+    if (m) return { field: name, root: m[1] as 'record' | 'previous' };
+  }
+  return null;
+}
+
 /**
  * Tier-4 type-soundness (#1928): detect a `record`-scoped expression that
  * type-checks structurally but faults a runtime operator overload because a
@@ -1074,46 +1127,85 @@ function offendingField(
  * `scope` selects how fields are bound: `'record'` (default) for
  * `record.<field>` sites; `'flattened'` for bare-field flow/automation
  * conditions.
+ *
+ * `bareShadowed` (flattened scope only, #29959): names the run binds BEFORE it
+ * flattens the record's fields — a flow's declared variables and node ids.
+ * `seedRunVariables` seeds those first and the flattening skips a bound name,
+ * so a bare read of one holds the variable, not the field: it stays `dyn`
+ * here. `record.<name>` still reads the field and stays typed.
  */
 export function firstTypeMismatch(
   source: string,
   fieldCelTypes: Readonly<Record<string, FieldCelType>>,
   scope: 'record' | 'flattened' = 'record',
-): { operator: string; operands: string; celType: FieldCelType; field: string | null; category: 'date-arith' | 'type-mismatch' } | null {
+  bareShadowed?: ReadonlySet<string>,
+): {
+  operator: string;
+  operands: string;
+  celType: FieldCelType;
+  field: string | null;
+  /** [#29959] Set when a flattened condition's mismatch is a `record.<field>` / `previous.<field>` read. */
+  root?: 'record' | 'previous';
+  category: 'date-arith' | 'type-mismatch';
+} | null {
   if (typeof source !== 'string' || !source.trim()) return null;
   // An all-`dyn` record can never fault an overload — skip the parse entirely.
   if (!Object.values(fieldCelTypes).some((t) => t === 'string' || t === 'bool' || t === 'timestamp')) return null;
-  try {
+  /** The faulting overload `source` meets in `env`'s typing, or `null`. */
+  const unsoundOverload = (env: Environment): RegExpExecArray | null => {
     // A null-guarded numeric branch (`cond ? n : null`) faults cel-js's ternary
     // unifier *before* the check reaches an inner date-arith overload; rewrite it
     // first (same pass the runtime uses) so the real fault surfaces (#3306).
-    const env = buildTypedEnv(fieldCelTypes, scope);
     const result = env.parse(rewriteNullableTernary(rewriteIndexedHas(source))).check?.() as
       | { valid?: boolean; error?: { message?: string } }
       | undefined;
     if (!result || result.valid !== false) return null;
-    const m = UNSOUND_OVERLOAD_RE.exec(result.error?.message ?? '');
-    if (!m) return null;
-    const operator = m[2];
-    const operands = `${m[1]} ${operator} ${m[3]}`;
-    // #3306 — date/duration ARITHMETIC against a number: always nulls, hard error.
-    if (ARITH_OPS.has(operator)
-      && ((DATE_CEL_TYPES.has(m[1]) && NUMERIC_CEL_TYPES.has(m[3]))
-        || (NUMERIC_CEL_TYPES.has(m[1]) && DATE_CEL_TYPES.has(m[3])))) {
+    return UNSOUND_OVERLOAD_RE.exec(result.error?.message ?? '');
+  };
+  /** The text/boolean operand type of a faulting overload, or `null`. */
+  const textOrBool = (m: RegExpExecArray): FieldCelType | null =>
+    m[1] === 'string' || m[1] === 'bool' ? (m[1] as FieldCelType)
+    : m[3] === 'string' || m[3] === 'bool' ? (m[3] as FieldCelType)
+    : null;
+  const bareTypes = scope === 'flattened' && bareShadowed && bareShadowed.size > 0
+    ? withoutNames(fieldCelTypes, bareShadowed)
+    : fieldCelTypes;
+  try {
+    const m = unsoundOverload(buildTypedEnv(bareTypes, scope));
+    if (m) {
+      const operator = m[2];
+      const operands = `${m[1]} ${operator} ${m[3]}`;
+      // #3306 — date/duration ARITHMETIC against a number: always nulls, hard error.
+      if (ARITH_OPS.has(operator)
+        && ((DATE_CEL_TYPES.has(m[1]) && NUMERIC_CEL_TYPES.has(m[3]))
+          || (NUMERIC_CEL_TYPES.has(m[1]) && DATE_CEL_TYPES.has(m[3])))) {
+        return {
+          operator, operands, celType: 'timestamp', category: 'date-arith',
+          field: offendingField(source, bareTypes, 'timestamp', scope),
+        };
+      }
+      // #1928 — text/boolean field in arithmetic/ordering against a number: warning.
+      const celType = textOrBool(m);
+      if (!celType) return null;
       return {
-        operator, operands, celType: 'timestamp', category: 'date-arith',
-        field: offendingField(source, fieldCelTypes, 'timestamp', scope),
+        operator, operands, celType, category: 'type-mismatch',
+        field: offendingField(source, bareTypes, celType, scope),
       };
     }
-    // #1928 — text/boolean field in arithmetic/ordering against a number: warning.
-    const celType: FieldCelType | null =
-      m[1] === 'string' || m[1] === 'bool' ? (m[1] as FieldCelType)
-      : m[3] === 'string' || m[3] === 'bool' ? (m[3] as FieldCelType)
-      : null;
-    if (!celType) return null;
+    // [#29959] A flattened condition reads the same field as `record.<field>`
+    // (and `previous.<field>`), which the bare typing above leaves `dyn`. A
+    // second pass types those roots too, for the text/boolean mismatch only:
+    // the same verdict the bare name gets. The date verdicts stay the bare
+    // name's alone, as before.
+    if (scope !== 'flattened' || !/(?<![\w$.])(?:record|previous)\s*\./.test(source)) return null;
+    const r = unsoundOverload(buildTypedEnv(bareTypes, 'flattened', false, fieldCelTypes));
+    const celType = r ? textOrBool(r) : null;
+    if (!r || !celType) return null;
+    const read = offendingRecordRead(source, fieldCelTypes, celType);
     return {
-      operator, operands, celType, category: 'type-mismatch',
-      field: offendingField(source, fieldCelTypes, celType, scope),
+      operator: r[2], operands: `${r[1]} ${r[2]} ${r[3]}`, celType, category: 'type-mismatch',
+      field: read?.field ?? null,
+      ...(read ? { root: read.root } : {}),
     };
   } catch {
     // A parse/other fault is the syntax checker's job (celEngine.compile); this