fix: #2305 emit Condition rule groups under conditionConfig

buildConditionNode emitted the rule group as a top-level `config.group`.
processActionConfig lifts only `condition` and `conditionConfig` out of the
config before rendering templates, and processTemplates does not recurse into
arrays, so `group.rules[0].leftOperand` kept its token. The leftover-literal
scan then found the survivor and failed closed, aborting the run before the
Condition node ever executed.

Three parts, which cannot ship apart:

1. The builder emits `conditionConfig: { group }`, matching the shape the
   scan factory builder has always produced.
2. A migration moves the key on rows already written. Seeded starters cannot
   be repaired from the editor, which persists conditionConfig but leaves the
   stale group in place, and the public hub rows insert with a fixed id and
   onConflictDoNothing() so they never pick up a fixture change.
3. The leftover-literal error names the field that carried the token. When an
   unread key holds a correctly spelled reference, the old message sent the
   reader to rewrite something that was never wrong.

Part 3 is not Condition-specific by design. data.config is deliberately an
open record, so there is no allowlist to validate against, but the path is
known at the point the token is found.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: bongbongcrypto

drizzle/0151_keep_2305_condition_group_to_condition_config.sql

@@ -0,0 +1,61 @@
+-- Issue #2305: move a Condition node's rule group from the unread top-level `group`
+-- key to `conditionConfig`, which is the shape the runtime reads.
+--
+-- lib/workflow/node-builders.ts emitted `data.config.group`. processActionConfig lifts
+-- only `condition` and `conditionConfig` out of the config before rendering templates,
+-- so `group.rules` kept its unrendered `{{...}}` tokens, and the leftover-literal scan
+-- that runs next found them and aborted the run before the Condition node executed.
+--
+-- The rows cannot be repaired from the editor: opening a seeded Condition node parses
+-- the `condition` string into a group and persists it as `conditionConfig`, but never
+-- deletes the stale top-level `group`, so the workflow still aborts. That is why the
+-- builder fix alone does not reach organizations provisioned so far, including the
+-- public hub rows, which insert with a fixed id and onConflictDoNothing() and are
+-- therefore never refreshed from the fixture.
+--
+-- Idempotent. A row whose Condition nodes already carry only `conditionConfig` is not
+-- matched. Where both keys exist the existing `conditionConfig` wins and only the stale
+-- `group` is dropped, so re-running changes nothing.
+--
+-- `updated_at` is deliberately left alone: this is a repair, not a user edit, and
+-- moving it would reorder every affected workflow in the user's list.
+
+UPDATE workflows AS w
+SET nodes = fixed.nodes
+FROM (
+  SELECT
+    src.id AS id,
+    jsonb_agg(
+      CASE
+        WHEN node #>> '{data,config,actionType}' = 'Condition'
+             AND jsonb_exists(node #> '{data,config}', 'group')
+        THEN jsonb_set(
+               node #- '{data,config,group}',
+               '{data,config,conditionConfig}',
+               CASE
+                 WHEN jsonb_exists(
+                        COALESCE(node #> '{data,config,conditionConfig}', '{}'::jsonb),
+                        'group'
+                      )
+                 THEN node #> '{data,config,conditionConfig}'
+                 ELSE COALESCE(node #> '{data,config,conditionConfig}', '{}'::jsonb)
+                      || jsonb_build_object('group', node #> '{data,config,group}')
+               END,
+               true
+             )
+        ELSE node
+      END
+      ORDER BY ord
+    ) AS nodes
+  FROM workflows AS src,
+       LATERAL jsonb_array_elements(src.nodes) WITH ORDINALITY AS elem(node, ord)
+  WHERE jsonb_typeof(src.nodes) = 'array'
+    AND EXISTS (
+      SELECT 1
+      FROM jsonb_array_elements(src.nodes) AS probe(node)
+      WHERE probe.node #>> '{data,config,actionType}' = 'Condition'
+        AND jsonb_exists(probe.node #> '{data,config}', 'group')
+    )
+  GROUP BY src.id
+) AS fixed
+WHERE w.id = fixed.id;

drizzle/meta/_journal.json

@@ -1058,6 +1058,13 @@
       "when": 1788517619226,
       "tag": "0150_keep_1333_summary_covering_index",
       "breakpoints": true
+    },
+    {
+      "idx": 151,
+      "version": "7",
+      "when": 1788517620226,
+      "tag": "0151_keep_2305_condition_group_to_condition_config",
+      "breakpoints": true
     }
   ]
-}
+}

lib/workflow/executor/template-resolution.ts

@@ -24,6 +24,13 @@ export type UnresolvedRef = {
   token: string;
   reason: UnresolvedReason;
   detail?: string;
+  /**
+   * Where in the rendered config the token was found, as a dotted path with
+   * bracket indices, for example `group.rules[0].leftOperand`. Present for
+   * leftover literals, which are the case where the token's own spelling is
+   * often correct and the field holding it is the actual fault.
+   */
+  path?: string;
 };
 
 export type TemplateResolutionTracker = {
@@ -63,7 +70,8 @@ export function recordUnresolved(
 export function scanForLeftoverLiterals(
   value: unknown,
   out: UnresolvedRef[],
-  depth = 0
+  depth = 0,
+  path = ""
 ): void {
   if (depth > 10 || out.length > 50) {
     return;
@@ -74,6 +82,7 @@ export function scanForLeftoverLiterals(
         token: match[0],
         reason: "literal-leftover",
         detail: "Reference left in rendered config; resolver did not match.",
+        path: path || undefined,
       });
       if (out.length > 50) {
         return;
@@ -82,14 +91,19 @@ export function scanForLeftoverLiterals(
     return;
   }
   if (Array.isArray(value)) {
-    for (const item of value) {
-      scanForLeftoverLiterals(item, out, depth + 1);
+    for (const [index, item] of value.entries()) {
+      scanForLeftoverLiterals(item, out, depth + 1, `${path}[${index}]`);
     }
     return;
   }
   if (value && typeof value === "object") {
-    for (const item of Object.values(value as Record<string, unknown>)) {
-      scanForLeftoverLiterals(item, out, depth + 1);
+    for (const [key, item] of Object.entries(value as Record<string, unknown>)) {
+      scanForLeftoverLiterals(
+        item,
+        out,
+        depth + 1,
+        path ? `${path}.${key}` : key
+      );
     }
   }
 }
@@ -129,7 +143,13 @@ function dedupeByToken(refs: UnresolvedRef[]): UnresolvedRef[] {
 
 function formatErrorMessage(unresolved: UnresolvedRef[]): string {
   const summaries = unresolved.slice(0, 5).map((ref) => {
-    return ref.detail ? `${ref.token} (${ref.detail})` : ref.token;
+    // Naming the field matters when the token itself is spelled correctly and the
+    // key holding it is the one the renderer never reached. Without it the message
+    // sends the reader to rewrite a reference that was never wrong.
+    const where = ref.path ? ` at ${ref.path}` : "";
+    return ref.detail
+      ? `${ref.token}${where} (${ref.detail})`
+      : `${ref.token}${where}`;
   });
   const more =
     unresolved.length > summaries.length

lib/workflow/node-builders.ts

@@ -92,7 +92,13 @@ export function buildConditionNode(
       config: {
         actionType: "Condition",
         condition: conditionConfig.condition,
-        group: conditionConfig.group,
+        // The runtime reads the rule group from `conditionConfig`, and
+        // `processActionConfig` lifts only `condition` and `conditionConfig` out
+        // before rendering templates. A top-level `group` is therefore never read,
+        // and its rules array carries unrendered tokens into the leftover-literal
+        // scan, which aborts the run. `lib/scan/factory/node-builders.ts` has always
+        // emitted the nested shape; this builder is the one that drifted.
+        conditionConfig: { group: conditionConfig.group },
       },
       status: "idle",
     },

tests/unit/template-fail-closed.test.ts

@@ -698,3 +698,73 @@ describe("extractTemplateParameters strict integration", () => {
     expect(tracker.unresolved[0]?.reason).toBe("no-path");
   });
 });
+
+describe("leftover literals name the field that carried them", () => {
+  // Issue #2305: a config key the renderer never reaches keeps its tokens, and the
+  // scan then reports the reference as unresolved. The reference is usually spelled
+  // correctly and the key above it is the fault, so the message has to say where.
+  const conditionConfigWithStaleGroup = {
+    actionType: "Condition",
+    condition: "resolved by its own path",
+    group: {
+      id: "group-1",
+      logic: "AND",
+      rules: [
+        {
+          id: "rule-1",
+          leftOperand: "{{@step-1:Get Aave Health Factor.healthFactor}}",
+          operator: "<",
+          rightOperand: "1500000000000000000",
+        },
+      ],
+    },
+  };
+
+  it("names the path through an array-valued key", () => {
+    const tracker = createTracker();
+    let message = "";
+    try {
+      assertResolved(tracker, conditionConfigWithStaleGroup, {
+        nodeId: "step-2",
+        nodeLabel: "Condition",
+        actionType: "Condition",
+      });
+    } catch (error) {
+      message = (error as Error).message;
+    }
+    expect(message).toMatch(UNRESOLVED_REF_MESSAGE);
+    expect(message).toContain("group.rules[0].leftOperand");
+    expect(message).toContain("{{@step-1:Get Aave Health Factor.healthFactor}}");
+  });
+
+  it("records the path on the ref itself", () => {
+    const tracker = createTracker();
+    try {
+      assertResolved(tracker, conditionConfigWithStaleGroup, {});
+    } catch (error) {
+      const { unresolved } = error as TemplateResolutionError;
+      expect(unresolved[0]?.path).toBe("group.rules[0].leftOperand");
+      expect(unresolved[0]?.reason).toBe("literal-leftover");
+    }
+  });
+
+  it("omits the path clause when the token sits at the root", () => {
+    const tracker = createTracker();
+    let message = "";
+    try {
+      assertResolved(tracker, "{{@step-1:Node.field}}", {});
+    } catch (error) {
+      message = (error as Error).message;
+    }
+    expect(message).toMatch(UNRESOLVED_REF_MESSAGE);
+    expect(message).not.toContain(" at ");
+  });
+
+  it("does not throw once the group is nested under conditionConfig", () => {
+    // processActionConfig lifts `conditionConfig` out before rendering, so its
+    // tokens never reach this scan. Simulate that by passing the config without it.
+    const { group, ...rest } = conditionConfigWithStaleGroup;
+    const repaired = { ...rest };
+    expect(() => assertResolved(createTracker(), repaired, {})).not.toThrow();
+  });
+});