fix(flows): author conditions as CEL envelopes so they actually evaluate

.changeset/flow-conditions-cel-envelopes.md

@@ -0,0 +1,33 @@
+---
+'hotcrm': patch
+---
+
+Make flow conditions actually evaluate (#562). Every `decision` node and
+conditional edge in `src/flows/` authored its condition as a bare string, and
+`AutomationEngine.evaluateCondition` only routes an `Expression` envelope
+(`{ dialect, source }`) to the CEL engine. A bare string fell through to a
+legacy template path that substitutes `{var}` braces and then compares both
+sides as strings — so no condition in the app was ever evaluated as an
+expression.
+
+The failure mode was not a uniform no-op, which is what made it dangerous.
+`existingStallTask == null` compared `'existingStallTask'` to `'null'` and was
+always false, so `opportunity_stagnation` selected the right stalled deals (once
+#489 was fixed) and then silently dropped every one of them at the idempotency
+gate — no notification, no follow-up task, and a `success` run record either
+way. In the other direction `record.rating >= 4` compared `'record.rating'` to
+`'4'`, and `'r' > '4'` is true, so `lead_assignment` pinned the Hot branch open
+and never took the Standard path.
+
+All 41 condition sites are now authored with the `P` tagged template from
+`@objectstack/spec`, which emits the envelope at authoring time. The condition
+sources are unchanged: they were already valid CEL — flow variables resolve by
+bare name and `record` is the triggering record, because the engine merges its
+variable map onto the CEL scope via `ctx.extra`. Only the envelope was missing.
+
+Note that `defineFlow()` would not have been enough on its own: it normalizes
+the typed edge `condition`, but a node's `config` is `z.record(z.unknown())`, so
+every start-node trigger gate would have stayed a bare string.
+
+A guard in `test/metadata-references.test.ts` fails on any condition that is not
+a CEL envelope, at either site, so the bare form cannot come back silently.

src/flows/campaign-enrollment.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -64,7 +65,7 @@ export const CampaignEnrollmentFlow: Flow = {
       // topping up a completed/aborted campaign would corrupt its final
       // snapshot metrics. (Status values: planning / in_progress / completed / aborted.)
       id: 'check_campaign_open', type: 'decision', label: 'Campaign Open?',
-      config: { condition: 'campaignRecord.status == "planning" || campaignRecord.status == "in_progress"' },
+      config: { condition: P`campaignRecord.status == "planning" || campaignRecord.status == "in_progress"` },
     },
     {
       id: 'query_leads', type: 'get_record', label: 'Find Eligible Leads',
@@ -101,7 +102,7 @@ export const CampaignEnrollmentFlow: Flow = {
             },
             {
               id: 'check_not_enrolled', type: 'decision', label: 'New Member?',
-              config: { condition: 'existingMember == null' },
+              config: { condition: P`existingMember == null` },
             },
             {
               id: 'create_campaign_member', type: 'create_record', label: 'Add to Campaign',
@@ -114,7 +115,7 @@ export const CampaignEnrollmentFlow: Flow = {
           edges: [
             { id: 'b1', source: 'find_existing_member', target: 'check_not_enrolled', type: 'default' },
             // Already enrolled → no edge → next lead.
-            { id: 'b2', source: 'check_not_enrolled', target: 'create_campaign_member', type: 'conditional', condition: 'existingMember == null', label: 'Enroll' },
+            { id: 'b2', source: 'check_not_enrolled', target: 'create_campaign_member', type: 'conditional', condition: P`existingMember == null`, label: 'Enroll' },
           ],
         },
       },
@@ -127,7 +128,7 @@ export const CampaignEnrollmentFlow: Flow = {
     { id: 'e2', source: 'screen_1', target: 'get_campaign', type: 'default' },
     { id: 'e3', source: 'get_campaign', target: 'check_campaign_open', type: 'default' },
     // Closed campaign → no edge → flow ends without enrolling.
-    { id: 'e4', source: 'check_campaign_open', target: 'query_leads', type: 'conditional', condition: 'campaignRecord.status == "planning" || campaignRecord.status == "in_progress"', label: 'Open' },
+    { id: 'e4', source: 'check_campaign_open', target: 'query_leads', type: 'conditional', condition: P`campaignRecord.status == "planning" || campaignRecord.status == "in_progress"`, label: 'Open' },
     { id: 'e5', source: 'query_leads', target: 'loop_leads', type: 'default' },
     { id: 'e6', source: 'loop_leads', target: 'end', type: 'default' },
   ],

src/flows/case-csat-followup.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -34,7 +35,7 @@ export const CaseCsatFollowupFlow: Flow = {
       config: {
         objectName: 'crm_case',
         triggerType: 'record-after-update',
-        condition: 'record.status == "closed" && (previous == null || previous.status != "closed")',
+        condition: P`record.status == "closed" && (previous == null || previous.status != "closed")`,
       },
     },
     {

src/flows/case-escalation.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -42,9 +43,8 @@ export const CaseEscalationFlow: Flow = {
         // the boolean is_closed suffers the SQLite `1 != true` trap above.
         // `escalated_date == null` additionally keeps a case that was already
         // escalated once (then reopened) from being escalated again.
-        condition:
-          'record.priority == "critical" && record.escalated_date == null'
-          + ' && record.status != "escalated" && record.status != "resolved" && record.status != "closed"',
+        condition: P`record.priority == "critical" && record.escalated_date == null
+          && record.status != "escalated" && record.status != "resolved" && record.status != "closed"`,
       },
     },
     {

src/flows/contact-welcome.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -31,7 +32,7 @@ export const ContactWelcomeFlow: Flow = {
       config: {
         objectName: 'crm_contact',
         triggerType: 'record-after-create',
-        condition: 'record.owner != null && record.email_opt_out != true',
+        condition: P`record.owner != null && record.email_opt_out != true`,
       },
     },
     {

src/flows/contract-renewal.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -58,7 +59,7 @@ export const ContractRenewalFlow: Flow = {
           nodes: [
             {
               id: 'check_notice_window', type: 'decision', label: 'Within Notice Window?',
-              config: { condition: 'timestamp(currentContract.end_date) <= daysFromNow(int(currentContract.renewal_notice_days))' },
+              config: { condition: P`timestamp(currentContract.end_date) <= daysFromNow(int(currentContract.renewal_notice_days))` },
             },
             {
               // Idempotency gate: the sweep matches the same contract every
@@ -79,7 +80,7 @@ export const ContractRenewalFlow: Flow = {
             },
             {
               id: 'check_not_reminded', type: 'decision', label: 'First Reminder?',
-              config: { condition: 'existingRenewalTask == null' },
+              config: { condition: P`existingRenewalTask == null` },
             },
             {
               id: 'create_renewal_task', type: 'create_record', label: 'Create Renewal Task',
@@ -108,7 +109,7 @@ export const ContractRenewalFlow: Flow = {
             },
             {
               id: 'check_auto_renewal', type: 'decision', label: 'Auto-Renewal On?',
-              config: { condition: 'currentContract.auto_renewal == true' },
+              config: { condition: P`currentContract.auto_renewal == true` },
             },
             {
               // Second gate: never open a second renewal opportunity while one
@@ -127,7 +128,7 @@ export const ContractRenewalFlow: Flow = {
             },
             {
               id: 'check_no_open_renewal', type: 'decision', label: 'No Open Renewal Deal?',
-              config: { condition: 'existingRenewalOpp == null' },
+              config: { condition: P`existingRenewalOpp == null` },
             },
             {
               id: 'create_renewal_opp', type: 'create_record', label: 'Open Renewal Opportunity',
@@ -149,14 +150,14 @@ export const ContractRenewalFlow: Flow = {
           edges: [
             // Only act when inside the per-contract notice window; gates with
             // no matching edge simply end the iteration, so the loop moves on.
-            { id: 'b1', source: 'check_notice_window', target: 'find_existing_task', type: 'conditional', condition: 'timestamp(currentContract.end_date) <= daysFromNow(int(currentContract.renewal_notice_days))', label: 'In window' },
+            { id: 'b1', source: 'check_notice_window', target: 'find_existing_task', type: 'conditional', condition: P`timestamp(currentContract.end_date) <= daysFromNow(int(currentContract.renewal_notice_days))`, label: 'In window' },
             { id: 'b2', source: 'find_existing_task', target: 'check_not_reminded', type: 'default' },
-            { id: 'b3', source: 'check_not_reminded', target: 'create_renewal_task', type: 'conditional', condition: 'existingRenewalTask == null', label: 'First reminder' },
+            { id: 'b3', source: 'check_not_reminded', target: 'create_renewal_task', type: 'conditional', condition: P`existingRenewalTask == null`, label: 'First reminder' },
             { id: 'b4', source: 'create_renewal_task', target: 'notify_owner', type: 'default' },
             { id: 'b5', source: 'notify_owner', target: 'check_auto_renewal', type: 'default' },
-            { id: 'b6', source: 'check_auto_renewal', target: 'find_existing_renewal_opp', type: 'conditional', condition: 'currentContract.auto_renewal == true', label: 'Auto-renew' },
+            { id: 'b6', source: 'check_auto_renewal', target: 'find_existing_renewal_opp', type: 'conditional', condition: P`currentContract.auto_renewal == true`, label: 'Auto-renew' },
             { id: 'b7', source: 'find_existing_renewal_opp', target: 'check_no_open_renewal', type: 'default' },
-            { id: 'b8', source: 'check_no_open_renewal', target: 'create_renewal_opp', type: 'conditional', condition: 'existingRenewalOpp == null', label: 'Open renewal deal' },
+            { id: 'b8', source: 'check_no_open_renewal', target: 'create_renewal_opp', type: 'conditional', condition: P`existingRenewalOpp == null`, label: 'Open renewal deal' },
           ],
         },
       },

src/flows/demo-bootstrap.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -120,7 +121,7 @@ export const DemoBootstrapFlow: Flow = {
     },
     {
       id: 'has_user', type: 'decision', label: 'Any user yet?',
-      config: { condition: 'vars.firstUser != null' },
+      config: { condition: P`vars.firstUser != null` },
     },
     ...TARGETS.flatMap((t) => [t.find, t.loop]),
     { id: 'end', type: 'end', label: 'End' },
@@ -134,12 +135,12 @@ export const DemoBootstrapFlow: Flow = {
       target: CHAIN[i + 1],
       type: 'default' as const,
       // The only branch: leaving `has_user` towards the first claim step.
-      ...(source === 'has_user' ? { condition: 'vars.firstUser != null', label: 'Yes' } : {}),
+      ...(source === 'has_user' ? { condition: P`vars.firstUser != null`, label: 'Yes' } : {}),
     })),
     // Nothing to claim before anyone exists — skip the whole chain.
     {
       id: 'e_nouser', source: 'has_user', target: 'end', type: 'default',
-      condition: 'vars.firstUser == null', label: 'No user yet',
+      condition: P`vars.firstUser == null`, label: 'No user yet',
     },
   ],
 };

src/flows/lead-assignment.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -40,7 +41,7 @@ export const LeadAssignmentFlow: Flow = {
     },
     {
       id: 'check_hot', type: 'decision', label: 'Hot Lead (rating ≥ 4)?',
-      config: { condition: 'record.rating >= 4' },
+      config: { condition: P`record.rating >= 4` },
     },
 
     // ── Hot path: 1-day SLA, high-severity alert ───────────────────
@@ -91,8 +92,8 @@ export const LeadAssignmentFlow: Flow = {
 
   edges: [
     { id: 'e1', source: 'start', target: 'check_hot', type: 'default' },
-    { id: 'e2', source: 'check_hot', target: 'sla_hot', type: 'conditional', condition: 'record.rating >= 4', label: 'Hot' },
-    { id: 'e3', source: 'check_hot', target: 'sla_std', type: 'conditional', condition: 'record.rating < 4', label: 'Standard' },
+    { id: 'e2', source: 'check_hot', target: 'sla_hot', type: 'conditional', condition: P`record.rating >= 4`, label: 'Hot' },
+    { id: 'e3', source: 'check_hot', target: 'sla_std', type: 'conditional', condition: P`record.rating < 4`, label: 'Standard' },
     { id: 'e4', source: 'sla_hot', target: 'notify_hot', type: 'default' },
     { id: 'e5', source: 'notify_hot', target: 'end', type: 'default' },
     { id: 'e6', source: 'sla_std', target: 'notify_std', type: 'default' },

src/flows/lead-conversion.flow.ts

@@ -1,5 +1,6 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
+import { P } from '@objectstack/spec';
 import type * as Automation from '@objectstack/spec/automation';
 type Flow = Automation.Flow;
 
@@ -56,7 +57,7 @@ export const LeadConversionFlow: Flow = {
     },
     {
       id: 'decision_account', type: 'decision', label: 'Account Already Exists?',
-      config: { condition: 'vars.matchedAccount != null' },
+      config: { condition: P`vars.matchedAccount != null` },
     },
     {
       // NEW-account branch. outputVariable is `createdAccount`; the assignment
@@ -100,7 +101,7 @@ export const LeadConversionFlow: Flow = {
     },
     {
       id: 'decision_contact', type: 'decision', label: 'Contact Already Exists?',
-      config: { condition: 'vars.matchedContact != null' },
+      config: { condition: P`vars.matchedContact != null` },
     },
     {
       // `accountId` is a bare id string from whichever account branch ran.
@@ -126,7 +127,7 @@ export const LeadConversionFlow: Flow = {
     },
     {
       id: 'decision_opportunity', type: 'decision', label: 'Create Opportunity?',
-      config: { condition: 'vars.createOpportunity == true' },
+      config: { condition: P`vars.createOpportunity == true` },
     },
     {
       id: 'create_opportunity', type: 'create_record', label: 'Create Opportunity',
@@ -189,21 +190,21 @@ export const LeadConversionFlow: Flow = {
     { id: 'e3', source: 'get_lead', target: 'find_account', type: 'default' },
     { id: 'e4', source: 'find_account', target: 'decision_account', type: 'default' },
     // Existing account → reuse; no account → create. Both converge on create_contact.
-    { id: 'e5', source: 'decision_account', target: 'use_existing_account', type: 'default', condition: 'vars.matchedAccount != null', label: 'Existing' },
-    { id: 'e6', source: 'decision_account', target: 'create_account', type: 'default', condition: 'vars.matchedAccount == null', label: 'New' },
+    { id: 'e5', source: 'decision_account', target: 'use_existing_account', type: 'default', condition: P`vars.matchedAccount != null`, label: 'Existing' },
+    { id: 'e6', source: 'decision_account', target: 'create_account', type: 'default', condition: P`vars.matchedAccount == null`, label: 'New' },
     { id: 'e7', source: 'create_account', target: 'use_new_account', type: 'default' },
     // Both account branches converge on the contact-dedupe lookup.
     { id: 'e8', source: 'use_new_account', target: 'find_contact', type: 'default' },
     { id: 'e9', source: 'use_existing_account', target: 'find_contact', type: 'default' },
     { id: 'e10', source: 'find_contact', target: 'decision_contact', type: 'default' },
     // Existing contact → reuse; none → create. Both converge on decision_opportunity.
-    { id: 'e11', source: 'decision_contact', target: 'use_existing_contact', type: 'default', condition: 'vars.matchedContact != null', label: 'Existing' },
-    { id: 'e12', source: 'decision_contact', target: 'create_contact', type: 'default', condition: 'vars.matchedContact == null', label: 'New' },
+    { id: 'e11', source: 'decision_contact', target: 'use_existing_contact', type: 'default', condition: P`vars.matchedContact != null`, label: 'Existing' },
+    { id: 'e12', source: 'decision_contact', target: 'create_contact', type: 'default', condition: P`vars.matchedContact == null`, label: 'New' },
     { id: 'e13', source: 'create_contact', target: 'use_new_contact', type: 'default' },
     { id: 'e14', source: 'use_new_contact', target: 'decision_opportunity', type: 'default' },
     { id: 'e15', source: 'use_existing_contact', target: 'decision_opportunity', type: 'default' },
-    { id: 'e16', source: 'decision_opportunity', target: 'create_opportunity', type: 'default', condition: 'vars.createOpportunity == true', label: 'Yes' },
-    { id: 'e17', source: 'decision_opportunity', target: 'no_opportunity', type: 'default', condition: 'vars.createOpportunity != true', label: 'No' },
+    { id: 'e16', source: 'decision_opportunity', target: 'create_opportunity', type: 'default', condition: P`vars.createOpportunity == true`, label: 'Yes' },
+    { id: 'e17', source: 'decision_opportunity', target: 'no_opportunity', type: 'default', condition: P`vars.createOpportunity != true`, label: 'No' },
     { id: 'e18', source: 'create_opportunity', target: 'use_new_opportunity', type: 'default' },
     { id: 'e18a', source: 'use_new_opportunity', target: 'mark_converted', type: 'default' },
     { id: 'e18b', source: 'no_opportunity', target: 'mark_converted', type: 'default' },