feat(validate): did-you-mean warnings for field typos in flow conditions (#1928)

.changeset/flow-condition-typo-warnings.md

@@ -0,0 +1,17 @@
+---
+"@objectstack/formula": patch
+"@objectstack/cli": patch
+---
+
+feat(validate): advisory did-you-mean warnings for likely field typos in flow conditions
+
+Adds a non-blocking warning channel to build-time expression validation (#1928
+tier 3). Flow / automation conditions flatten the record's fields to top-level,
+so a bare `status` is correct — but a bare NON-field identifier is either a flow
+variable or a typo. When it is a near-miss of a known field (edit distance), the
+build now emits a `did you mean \`status\`?` warning instead of staying silent,
+WITHOUT failing the build (a genuine flow variable won't be close to a field
+name, so it stays quiet). `ExprValidationResult` gains a `warnings` array and
+`ExprIssue` a `severity`; `objectstack compile` prints warnings and only fails on
+errors. This closes the silent-skip gap for misspelled trigger-condition fields
+(the #1877 family) without the false-positive risk of a hard gate.

packages/cli/src/commands/compile.ts

@@ -135,19 +135,29 @@ export default class Compile extends Command {
       //     located, corrective message instead of a silent runtime `false`.
       if (!flags.json) printStep('Validating expressions (ADR-0032)...');
       const exprIssues = validateStackExpressions(result.data as Record<string, unknown>);
-      if (exprIssues.length > 0) {
+      const exprErrors = exprIssues.filter((i) => i.severity !== 'warning');
+      const exprWarnings = exprIssues.filter((i) => i.severity === 'warning');
+      if (exprErrors.length > 0) {
         if (flags.json) {
-          console.log(JSON.stringify({ success: false, error: 'expression validation failed', issues: exprIssues }));
+          console.log(JSON.stringify({ success: false, error: 'expression validation failed', issues: exprErrors, warnings: exprWarnings }));
           this.exit(1);
         }
         console.log('');
-        printError(`Expression validation failed (${exprIssues.length} issue${exprIssues.length > 1 ? 's' : ''})`);
-        for (const i of exprIssues.slice(0, 50)) {
+        printError(`Expression validation failed (${exprErrors.length} issue${exprErrors.length > 1 ? 's' : ''})`);
+        for (const i of exprErrors.slice(0, 50)) {
           console.log(`  • ${i.where}: ${i.message}`);
           console.log(`      source: \`${i.source}\``);
         }
         this.exit(1);
       }
+      // Advisory expression warnings (#1928 tier 3) — surfaced, never fatal.
+      if (exprWarnings.length > 0 && !flags.json) {
+        printWarning(`Expression warnings (${exprWarnings.length})`);
+        for (const i of exprWarnings.slice(0, 50)) {
+          console.log(`  • ${i.where}: ${i.message}`);
+          console.log(`      source: \`${i.source}\``);
+        }
+      }
 
       // 3c. Widget-binding diagnostics (issues #1719/#1721) — semantic checks
       //     that need the widget's `dataset` reference resolved to its dataset

packages/cli/src/utils/validate-expressions.test.ts

@@ -191,5 +191,46 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
       });
       expect(issues).toHaveLength(0);
     });
+
+    // #1928 tier 3 — a likely field typo in a flow condition is a non-blocking warning.
+    it('warns (severity=warning) on a likely field typo in a flow condition', () => {
+      const issues = validateStackExpressions({
+        objects: [{ name: 'crm_opportunity', fields: { stage: { type: 'select' }, amount: { type: 'currency' } } }],
+        flows: [{
+          name: 'opp_won',
+          nodes: [
+            { id: 'start', type: 'start', config: { objectName: 'crm_opportunity', condition: 'stagee == "closed_won"' } },
+          ],
+          edges: [],
+        }],
+      });
+      expect(issues).toHaveLength(1);
+      expect(issues[0].severity).toBe('warning');
+      expect(issues[0].message).toMatch(/did you mean `stage`/);
+    });
+
+    it('does not warn when the bare ref is far from any field (likely a flow variable)', () => {
+      const issues = validateStackExpressions({
+        objects: [{ name: 'crm_opportunity', fields: { stage: { type: 'select' } } }],
+        flows: [{
+          name: 'renewal',
+          nodes: [{ id: 'start', type: 'start', config: { objectName: 'crm_opportunity' } }],
+          edges: [{ id: 'e1', source: 'start', target: 'end', condition: 'expiring_deals.length > 0' }],
+        }],
+      });
+      expect(issues).toHaveLength(0);
+    });
+
+    it('tags record-scoped bare-ref issues as errors', () => {
+      const issues = validateStackExpressions({
+        objects: [{
+          name: 'crm_lead',
+          fields: { lead_score: { type: 'number' } },
+          validations: [{ name: 'r', expression: 'lead_score > 100' }],
+        }],
+      });
+      expect(issues).toHaveLength(1);
+      expect(issues[0].severity).toBe('error');
+    });
   });
 });

packages/cli/src/utils/validate-expressions.ts

@@ -20,6 +20,12 @@ export interface ExprIssue {
   where: string;
   message: string;
   source: string;
+  /**
+   * `error` fails the build (e.g. a bare ref in a record-scoped formula). `warning`
+   * is advisory and never fails it (e.g. a possible field typo in a flattened flow
+   * condition, which might be a flow variable). Absent ⇒ treat as `error`.
+   */
+  severity?: 'error' | 'warning';
 }
 
 type AnyRec = Record<string, unknown>;
@@ -67,7 +73,8 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
     const fields = objectName ? fieldIndex.get(objectName) : undefined;
     const res = validateExpression('predicate', raw as string | { dialect?: string; source?: string },
       objectName ? { objectName, fields, scope } : { scope });
-    for (const e of res.errors) issues.push({ where, message: e.message, source: e.source });
+    for (const e of res.errors) issues.push({ where, message: e.message, source: e.source, severity: 'error' });
+    for (const w of res.warnings) issues.push({ where, message: w.message, source: w.source, severity: 'warning' });
   };
 
   // ── Flows ──────────────────────────────────────────────────────────
@@ -146,9 +153,9 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
         // `record`-scoped — `record.<field>`, never bare — so flag bare refs (#1928).
         const res = validateExpression('value', f.formula as string | { dialect?: string; source?: string },
           objectName ? { objectName, fields: fieldIndex.get(objectName), scope: 'record' } : { scope: 'record' });
-        for (const e of res.errors) {
-          issues.push({ where: `object '${objectName}' · field '${(f.name as string) ?? '?'}' formula`, message: e.message, source: e.source });
-        }
+        const fieldWhere = `object '${objectName}' · field '${(f.name as string) ?? '?'}' formula`;
+        for (const e of res.errors) issues.push({ where: fieldWhere, message: e.message, source: e.source, severity: 'error' });
+        for (const w of res.warnings) issues.push({ where: fieldWhere, message: w.message, source: w.source, severity: 'warning' });
       }
     }
   }

packages/formula/src/cel-engine.ts

@@ -60,9 +60,7 @@ const SCOPE_ROOTS = [
  * bare field references. It reuses the real stdlib so function calls don't fault;
  * only undeclared *variables* do. Built once — `parse`/`check` do not mutate it.
  */
-let scopedEnv: Environment | undefined;
-function getScopedEnv(): Environment {
-  if (scopedEnv) return scopedEnv;
+function buildScopedEnv(knownFields: readonly string[]): Environment {
   const env = new Environment({
     unlistedVariablesAreDyn: false,
     enableOptionalTypes: true,
@@ -72,10 +70,20 @@ function getScopedEnv(): Environment {
   for (const root of SCOPE_ROOTS) {
     try { env.registerVariable(root, 'map'); } catch { /* duplicate — ignore */ }
   }
-  scopedEnv = env;
+  // `knownFields` are declared as `dyn` so they (and member/arith/compare on
+  // them) never fault — only a genuinely-undeclared top-level identifier does.
+  // Empty for a record-scope site (any bare field is a bug); the trigger
+  // object's fields for a flattened flow condition (only a NON-field bare ref —
+  // a typo or flow variable — is then interesting).
+  for (const field of knownFields) {
+    try { env.registerVariable(field, 'dyn'); } catch { /* duplicate / reserved — ignore */ }
+  }
   return env;
 }
 
+// Roots-only env reused for the common record-scope check (no per-call rebuild).
+let recordScopeEnv: Environment | undefined;
+
 /**
  * In a `record`-scoped CEL site — a `Field.formula` or an object validation
  * predicate — the evaluation scope binds only the `record`/`previous`/… *namespaces*
@@ -89,10 +97,16 @@ function getScopedEnv(): Environment {
  * automation conditions, where the record's fields ARE flattened to top-level
  * and bare references are correct.
  */
-export function detectBareReference(source: string): string | null {
+export function firstUndeclaredReference(
+  source: string,
+  knownFields: readonly string[] = [],
+): string | null {
   if (typeof source !== 'string' || !source.trim()) return null;
   try {
-    const result = getScopedEnv().parse(source).check?.() as
+    const env = knownFields.length === 0
+      ? (recordScopeEnv ??= buildScopedEnv([]))
+      : buildScopedEnv(knownFields);
+    const result = env.parse(source).check?.() as
       | { valid: boolean; error?: { message?: string } }
       | undefined;
     if (result && result.valid === false) {
@@ -106,6 +120,11 @@ export function detectBareReference(source: string): string | null {
   return null;
 }
 
+/** @deprecated use {@link firstUndeclaredReference} with no fields. */
+export function detectBareReference(source: string): string | null {
+  return firstUndeclaredReference(source);
+}
+
 /** Coerce cel-js's BigInt-flavored return into spec-friendly JS values. */
 function coerce(value: unknown): unknown {
   if (typeof value === 'bigint') {

packages/formula/src/validate.test.ts

@@ -119,6 +119,40 @@ describe('validateExpression (ADR-0032)', () => {
     });
   });
 
+  // #1928 tier 3 — flattened flow conditions reference fields bare, so a bare
+  // ref is not an error. A bare NON-field that is a near-miss of a known field
+  // is a likely typo → non-blocking warning (ok stays true).
+  describe('flow-condition typo warnings (#1928 tier 3)', () => {
+    const fields = ['stage', 'amount', 'status'] as const;
+
+    it('warns (does not error) on a likely field typo in a flattened condition', () => {
+      const r = validateExpression('predicate', 'stagee == "closed_won"', { objectName: 'crm_opportunity', fields, scope: 'flattened' });
+      expect(r.ok).toBe(true);
+      expect(r.errors).toHaveLength(0);
+      expect(r.warnings).toHaveLength(1);
+      expect(r.warnings[0].message).toMatch(/`stagee` is not a field/);
+      expect(r.warnings[0].message).toMatch(/did you mean `stage`/);
+    });
+
+    it('does not warn on a correct bare field reference', () => {
+      const r = validateExpression('predicate', 'stage == "closed_won" && previous.stage != "closed_won"', { objectName: 'crm_opportunity', fields, scope: 'flattened' });
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(0);
+    });
+
+    it('does not warn on a flow variable that is far from any field name', () => {
+      const r = validateExpression('predicate', 'expiring_deals.length > 0', { objectName: 'crm_opportunity', fields, scope: 'flattened' });
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(0);
+    });
+
+    it('emits no warnings without a field list (nothing to compare against)', () => {
+      const r = validateExpression('predicate', 'stagee == "x"', { scope: 'flattened' });
+      expect(r.ok).toBe(true);
+      expect(r.warnings).toHaveLength(0);
+    });
+  });
+
   describe('introspection', () => {
     it('reports the dialect + scope for a field role', () => {
       expect(expectedDialect('predicate')).toBe('cel');

packages/formula/src/validate.ts

@@ -17,7 +17,7 @@
  * This validator detects that specific mistake and returns the exact fix.
  */
 
-import { celEngine, detectBareReference } from './cel-engine';
+import { celEngine, firstUndeclaredReference } from './cel-engine';
 import { templateEngine } from './template-engine';
 
 export type FieldRole = 'predicate' | 'value' | 'template';
@@ -46,9 +46,11 @@ export interface ExprSchemaHint {
    *                    it MUST be written `record.amount`. We flag bare refs.
    *  - `'flattened'` → the record's own fields are spread to top-level alongside
    *                    flow variables (flow / automation conditions), so bare
-   *                    `status` is correct. We do NOT flag bare refs here — flow
-   *                    variables are not schema-knowable, so a bare identifier
-   *                    can't be soundly told apart from a typo. (Default.)
+   *                    `status` is correct and is NOT an error. Flow variables
+   *                    are not schema-knowable, so a non-field bare identifier
+   *                    can't be soundly told apart from a typo — but when one is
+   *                    a near-miss of a known field we emit a non-blocking
+   *                    did-you-mean *warning*. (Default.)
    */
   scope?: 'record' | 'flattened';
 }
@@ -63,6 +65,13 @@ export interface ExprValidationError {
 export interface ExprValidationResult {
   ok: boolean;
   errors: ExprValidationError[];
+  /**
+   * Non-blocking advisories (#1928 tier 3): a likely-typo'd field reference in a
+   * flattened flow condition. Never affects `ok` — callers surface these without
+   * failing the build, since a bare identifier there may legitimately be a flow
+   * variable.
+   */
+  warnings: ExprValidationError[];
 }
 
 /** A bare `{x}` that is NOT part of a `{{x}}` mustache hole. */
@@ -149,14 +158,15 @@ export function validateExpression(
 ): ExprValidationResult {
   const { dialect, source } = toSource(input);
   const errors: ExprValidationError[] = [];
-  if (!source.trim()) return { ok: true, errors };
+  const warnings: ExprValidationError[] = [];
+  if (!source.trim()) return { ok: true, errors, warnings };
 
   if (role === 'template') {
     // Templates must be the `template` dialect (or untyped string). Reject a
     // CEL envelope mistakenly placed in a text field.
     if (dialect && dialect !== 'template') {
       errors.push({ source, message: `expected a text template but got a \`${dialect}\` expression.` });
-      return { ok: false, errors };
+      return { ok: false, errors, warnings };
     }
     const compiled = templateEngine.compile(source);
     if (!compiled.ok) {
@@ -165,13 +175,13 @@ export function validateExpression(
     // A single `{x}` in a template is the legacy/deprecated form (ADR-0032 §3).
     const hint = SINGLE_BRACE_RE.test(source) ? bracesHintForTemplate(source) : null;
     if (hint) errors.push({ source, message: hint });
-    return { ok: errors.length === 0, errors };
+    return { ok: errors.length === 0, errors, warnings };
   }
 
   // predicate | value → CEL
   if (dialect && dialect !== 'cel') {
     errors.push({ source, message: `expected a CEL expression but got a \`${dialect}\` dialect.` });
-    return { ok: false, errors };
+    return { ok: false, errors, warnings };
   }
   const compiled = celEngine.compile(source);
   if (!compiled.ok) {
@@ -184,11 +194,10 @@ export function validateExpression(
     });
   } else {
     checkFieldExistence(source, schema, errors);
-    // In a `record`-scoped site a bare top-level identifier is a silent bug —
-    // it must be `record.<field>` (#1928). Only flagged here; flow/automation
-    // conditions (`scope: 'flattened'`, the default) legitimately use bare refs.
     if (schema?.scope === 'record') {
-      const bare = detectBareReference(source);
+      // In a `record`-scoped site a bare top-level identifier is a silent bug —
+      // it must be `record.<field>` (#1928). Hard error.
+      const bare = firstUndeclaredReference(source);
       if (bare) {
         errors.push({
           source,
@@ -198,9 +207,28 @@ export function validateExpression(
             `expression silently evaluates to null. Write \`record.${bare}\`.`,
         });
       }
+    } else if (schema?.fields && schema.fields.length > 0) {
+      // Flattened flow/automation condition: the record's fields ARE bound at
+      // top-level, so a bare ref is normally correct. But a *non-field* bare
+      // identifier is either a flow variable or a typo. When it is a near-miss
+      // of a known field, warn (did-you-mean) WITHOUT failing the build —
+      // a genuine flow variable won't be edit-distance-close to a field. (#1928)
+      const unknown = firstUndeclaredReference(source, schema.fields);
+      if (unknown) {
+        const suggestion = nearest(unknown, schema.fields);
+        if (suggestion) {
+          warnings.push({
+            source,
+            message:
+              `\`${unknown}\` is not a field of \`${schema.objectName ?? 'the trigger object'}\` — ` +
+              `did you mean \`${suggestion}\`? (flow conditions reference fields bare, e.g. \`${suggestion} == …\`). ` +
+              `If \`${unknown}\` is a flow variable this is safe to ignore.`,
+          });
+        }
+      }
     }
   }
-  return { ok: errors.length === 0, errors };
+  return { ok: errors.length === 0, errors, warnings };
 }
 
 function bracesHintForTemplate(source: string): string {