fix(spec): let a schemaless node own an expression-ledger entry (#4439)

.changeset/decision-branch-routing-enforced.md

@@ -74,6 +74,21 @@ sibling — the actual hole), `flow-default-edge-with-condition` and
 Both of the first two fire on the pre-fix `convert-lead.flow.ts` and are silent
 after it.
 
+## Effect on flows that already exist
+
+Enforcing `isDefault` changes how a **stored** flow behaves, and the flows it
+changes are mostly Studio's own. `objectui`'s flow edge inspector has always
+written `isDefault: true` when you bind an out-edge to a decision's default/else
+branch — into a key with zero readers, so that edge ran unconditionally, in
+parallel with whichever branch actually matched. Those flows now take exactly
+one branch. That is the fix, but it is a behaviour change on existing data
+rather than only on newly authored metadata, so it is worth knowing before
+upgrading: a flow that quietly ran two paths will now run one.
+
+Nothing changes for an edge that never carried the marker — `isDefault` defaults
+to `false`, and an ordinary unconditional out-edge still fans out in parallel
+exactly as before.
+
 ## The example app
 
 `crm_convert_lead_wizard`'s guard is now a plain exclusive gateway: the

.changeset/schemaless-node-expression-ledger.md

@@ -0,0 +1,76 @@
+---
+"@objectstack/spec": minor
+"@objectstack/service-automation": patch
+"@objectstack/lint": patch
+---
+
+fix(spec): a node that publishes no descriptor configSchema can now own an expression-ledger entry (#4439)
+
+`FLOW_NODE_EXPRESSION_PATHS` is the #4027 ledger that tells `registerFlow` and
+`objectstack validate` which config keys hold expressions, and in which dialect.
+Its ratchet (`config-expression-ledger.test.ts`) derives what it expects from
+descriptor `configSchema` `xExpression` markers, and fails in **both**
+directions — an undeclared marker, or a ledger entry nothing declares.
+
+`decision` / `script` / `subflow` publish **no** descriptor `configSchema` on
+purpose: a published partial schema would drop the editors their hand-written
+Studio forms need (the #4210 incident), so their contract lives in
+`schemaless-node-config.zod.ts`. Those two rules compose into a hole — an
+expression slot on a schemaless node is structurally unreachable by the ratchet,
+and because the reverse direction rejects unclaimed entries, it cannot be
+entered by hand either.
+
+`decision.conditions[].expression` sat in that hole. Its own schema says
+*"Bare CEL predicate deciding this branch"* and its own comment names `{…}` as
+the #1491 trap, and no validator walked it — so `{lead_record.status} ==
+'converted'` passed `tsc`, passed `objectstack validate`, passed registration.
+#4414 made that fail loudly at run time; this makes it fail at build time,
+which is the delay #4027 exists to remove.
+
+## The fix
+
+The ratchet now reads **both** declaration channels:
+
+- **descriptor `configSchema`** — unchanged, enumerated from the live registry;
+- **`schemaless-node-config.zod.ts`** — the marker rides
+  `.meta({ xExpression })` through `z.toJSONSchema`, the same channel
+  `loop.collection` has used since objectui#2670.
+
+Spec hands the second channel over as JSON Schema
+(`getSchemalessNodeConfigJsonSchemas()`, memoized, `input` mode — the shape a
+descriptor's `configSchema` already is), so the ratchet walks both with the
+*same* function. No second notion of "a declared expression property", which is
+the duplication a ledger exists to remove, and no `zod` dependency added to
+`service-automation`. Each channel is separately asserted non-empty, so a broken
+derivation on one side cannot hide behind the other's results.
+
+`SCHEMALESS_NODE_CONFIG_SCHEMAS` is also exported for anything else that needs
+to reason about all node config contracts. Additive — objectui's
+`flow-node-config` reconciliation imports each schema by name and is unaffected.
+
+## The sweep
+
+The other schemaless slots were checked and deliberately carry no marker:
+`script.template` is a template **id**, not a body; `script.inputs` /
+`script.variables` / `subflow.input` are values that interpolate `{token}` —
+text-with-holes, the shape essentially every node config string has, already
+covered generically by `validate-flow-template-paths` and the CLI flow linter.
+A `flow-template` ledger entry means something narrower: a *reference that must
+resolve to a value*, like `loop.collection`. So `decision.conditions[]
+.expression` is the only genuinely declared expression slot on the class — now
+recorded in the ledger's header so it is not re-derived.
+
+## Docs corrected
+
+The flows guide taught the **wrong dialect** for decision predicates in three
+places (`'{order_amount} > 10000'`), plus a "braces missing in a decision
+expression" warning that inverted after #4414 — and `FlowNodeSchema`'s own
+`@example` did the same. All corrected to bare CEL, with the history stated so
+an author with a braced predicate knows what changed and why their build now
+fails. The dialect table drops from three dialects to two: predicates never take
+braces, values always do.
+
+Verified: 13 new/updated tests across the ratchet, the engine's registration
+pass and `@objectstack/lint` (including the exact app-crm predicate rejected at
+both `registerFlow` and `objectstack validate`); `pnpm build`, `pnpm typecheck`
+(122 tasks), `pnpm lint` and `check:docs` clean.

content/docs/automation/flows.mdx

@@ -31,7 +31,8 @@ const approvalFlow = {
       type: 'decision',
       label: 'Check Amount',
       config: {
-        // decision expressions are bare CEL, like every other condition — no braces
+        // decision expressions are bare CEL, like every other condition — no
+        // braces; see Expressions in flows
         conditions: [
           { label: 'High Value', expression: 'order_amount > 10000' },
           { label: 'Standard', expression: 'order_amount <= 10000' },
@@ -168,6 +169,7 @@ or missing-`required` violation (#4277). A node type that publishes no
   type: 'decision',
   label: 'Check Status',
   config: {
+    // Bare CEL — the labels must match this node's out-edge labels exactly.
     conditions: [
       { label: 'Approved', expression: "status == 'approved'" },
       { label: 'Rejected', expression: "status == 'rejected'" },
@@ -782,9 +784,16 @@ sibling), `flow-default-edge-with-condition` and `flow-multiple-default-edges`.
 
 <Callout type="warn">
 A decision node that declares **no** `conditions` reports no branch at all — it
-is a plain gateway and its out-edges do the routing. Do not declare both:
-`config.conditions` *and* per-edge `condition`s on the same node means the node
-picks a branch, and then that branch's edge re-decides.
+is a plain gateway and its out-edges do the routing.
+
+Declaring **both** — `config.conditions` *and* per-edge `condition`s — is
+redundant but not wrong: the node picks a branch, and then that branch's edge
+re-decides with the same predicate. The Studio flow designer emits exactly this
+(it copies each branch's expression and label onto the edge it wires), and it
+routes correctly because the two are kept in sync by construction. Hand-written
+metadata has no such guarantee, which is the whole of #4414: when the two
+disagree, the node's branch wins the narrowing and the edge's predicate decides
+what actually runs. If you are writing the flow by hand, pick one.
 </Callout>
 
 ### Fault edges — handling a failed node
@@ -979,7 +988,7 @@ condition is CEL; braces are for values.**
 |:---|:---|:---|:---|
 | Start-node `condition` | **CEL** (bare, no braces) | `record.amount > 500` | `record.*`, `previous.*`, bare field names, `vars.*` |
 | Edge `condition` | **CEL** (bare, no braces) | `record.status == 'open'` | same as above |
-| Decision-node `conditions[].expression` | **CEL** (bare, no braces) | `order_amount > 10000` | same as above |
+| Decision-node `conditions[].expression` | **CEL** (bare, no braces) | `order_amount > 10000` | flow variables by name, and `vars.*` |
 | Field values in `create_record` / `update_record` | **Interpolation** (braces required) | `'Follow up on {record.name}'`, `'{TODAY() + 7}'` | `{var}`, `{var.path}`, `{$User.Id}`, `{$User.Email}`, `{NOW()}`, `{TODAY()}`, `{TODAY() + 90}` (whole days) |
 
 <Callout type="warn">
@@ -1001,11 +1010,19 @@ a field access on an object variable is not one. Both reported `success`. They
 are bare CEL now, so the spellings in the table above are the correct ones and
 `lead_record.status == 'converted'` resolves the field.
 
-The `{var}` form still works where it always did — `{amount} > 100`,
-`{status} == 'active'` — but the two ways it used to answer `false` without
-saying so are now **loud errors** naming the reference: a `{…}` hole that
-matches no flow variable, and a substituted value that is neither a boolean, a
-number, nor part of a comparison (#4336).
+The `{var}` form still works where a condition is a plain authored string — a
+start node's `config.condition`: `{amount} > 100`, `{status} == 'active'`. The
+two ways it used to answer `false` without saying so are now **loud errors**
+naming the reference: a `{…}` hole that matches no flow variable, and a
+substituted value that is neither a boolean, a number, nor part of a comparison
+(#4336).
+
+**A decision's `conditions[].expression` is the exception — it is always CEL.**
+The slot is declared bare CEL and is on the expression ledger as a predicate
+(#4439), so a braced spelling there is not the `{var}` dialect but a build
+failure: `os build` and `registerFlow` reject it, naming
+`config.conditions[N].expression`. That is deliberate — the alternative is a
+build that refuses what run time would happily execute.
 </Callout>
 
 CEL conditions that fail to evaluate raise an error and stop the run — they

packages/lint/src/validate-expressions.test.ts

@@ -578,6 +578,43 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
       expect(issues).toHaveLength(0);
     });
 
+    /**
+     * #4439 — `decision.conditions[].expression` reaches the ledger through the
+     * SCHEMALESS channel (`decision` publishes no descriptor `configSchema`, so
+     * the marker rides `.meta({ xExpression })` on the Zod contract). Until then
+     * the ratchet could only see descriptor-declared slots, so this predicate —
+     * documented bare CEL, evaluated as bare CEL since #4414 — was checked by
+     * nobody and a `{…}` spelling passed `objectstack validate`.
+     */
+    const decisionFlow = (expression: string) => ({
+      objects,
+      flows: [{
+        name: 'convert_lead',
+        nodes: [
+          { id: 'start', type: 'start', config: { objectName: 'crm_lead' } },
+          { id: 'check', type: 'decision', config: { conditions: [{ label: 'Yes', expression }] } },
+        ],
+        edges: [],
+      }],
+    });
+
+    it('flags a `{var}` template dialect in a decision branch expression (#4439)', () => {
+      // The exact predicate app-crm shipped (#4414).
+      const issues = validateStackExpressions(decisionFlow("{lead_record.status} == 'converted'"));
+      const found = issues.filter(i => i.where.includes('conditions[0].expression'));
+      expect(found).toHaveLength(1);
+      expect(found[0].severity).toBe('error');
+      expect(found[0].where).toContain("flow 'convert_lead'");
+      expect(found[0].where).toContain("node 'check'");
+      expect(found[0].where).toContain('decision branch expression');
+      expect(found[0].source).toBe("{lead_record.status} == 'converted'");
+    });
+
+    it('passes the corrected bare-CEL decision predicate (#4439)', () => {
+      const issues = validateStackExpressions(decisionFlow("lead_record.status == 'converted'"));
+      expect(issues.filter(i => i.where.includes('conditions'))).toHaveLength(0);
+    });
+
     it('leaves a correct single-brace loop collection alone', () => {
       // `loop.collection` is the single-brace `{var}` flow-interpolation dialect,
       // where braces are CORRECT. It is recorded in the ledger as `flow-template`

packages/services/service-automation/src/builtin/config-expression-ledger.test.ts

@@ -28,6 +28,7 @@
 import { describe, it, expect } from 'vitest';
 import {
   FLOW_NODE_EXPRESSION_PATHS,
+  getSchemalessNodeConfigJsonSchemas,
   resolveFlowNodeExpressions,
   type FlowNodeExpressionRole,
 } from '@objectstack/spec/automation';
@@ -93,29 +94,67 @@ function collectExpressionProps(
 const engine = new AutomationEngine(silentLogger());
 installBuiltinNodes(engine, ctx());
 
-/** Every declared expression slot, derived from the live descriptors. */
-function declaredFromDescriptors(): { nodeType: string; path: string; role: FlowNodeExpressionRole }[] {
-  const found: { nodeType: string; path: string; role: FlowNodeExpressionRole }[] = [];
+type DeclaredSlot = { nodeType: string; path: string; role: FlowNodeExpressionRole };
+
+/** Resolve an `xExpression` marker to its ledger role, failing loudly on an unknown one. */
+function roleOf(nodeType: string, path: string, marker: string): FlowNodeExpressionRole {
+  const role = ROLE_BY_MARKER[marker];
+  expect(
+    role,
+    `${nodeType}.${path} declares an unknown xExpression marker '${marker}' — ` +
+    `add it to ROLE_BY_MARKER and teach the validators which dialect it takes`,
+  ).toBeDefined();
+  return role!;
+}
+
+/** Declared expression slots on builtins that publish a descriptor `configSchema`. */
+function declaredFromDescriptors(): DeclaredSlot[] {
+  const found: DeclaredSlot[] = [];
   for (const descriptor of engine.getActionDescriptors()) {
     const schema = descriptor.configSchema as SchemaNode | undefined;
     for (const { path, marker } of collectExpressionProps(schema)) {
-      const role = ROLE_BY_MARKER[marker];
-      expect(
-        role,
-        `${descriptor.type}.${path} declares an unknown xExpression marker '${marker}' — ` +
-        `add it to ROLE_BY_MARKER and teach the validators which dialect it takes`,
-      ).toBeDefined();
-      found.push({ nodeType: descriptor.type, path, role: role! });
+      found.push({ nodeType: descriptor.type, path, role: roleOf(descriptor.type, path, marker) });
+    }
+  }
+  return found;
+}
+
+/**
+ * Declared expression slots on the builtins that publish NO descriptor
+ * `configSchema` (#4439).
+ *
+ * `script` / `subflow` / `decision` keep their contract in
+ * `schemaless-node-config.zod.ts` on purpose, so deriving only from descriptors
+ * made their expression slots structurally unreachable by this ratchet — and
+ * because the reverse direction fails on a ledger entry nothing declares, they
+ * could not be entered by hand either. `decision.conditions[].expression` sat
+ * in that hole.
+ *
+ * Spec hands these over as JSON Schema — the same shape a descriptor's
+ * `configSchema` is — so the marker walk below is literally the same function.
+ * No second notion of "a declared expression property", which is the
+ * duplication a ledger exists to remove.
+ */
+function declaredFromSchemalessConfigs(): DeclaredSlot[] {
+  const found: DeclaredSlot[] = [];
+  for (const [nodeType, json] of Object.entries(getSchemalessNodeConfigJsonSchemas())) {
+    for (const { path, marker } of collectExpressionProps(json as SchemaNode)) {
+      found.push({ nodeType, path, role: roleOf(nodeType, path, marker) });
     }
   }
   return found;
 }
 
+/** Every declared expression slot, from BOTH declaration channels. */
+function declaredEverywhere(): DeclaredSlot[] {
+  return [...declaredFromDescriptors(), ...declaredFromSchemalessConfigs()];
+}
+
 const key = (e: { nodeType: string; path: string; role: string }) => `${e.nodeType}.${e.path} (${e.role})`;
 
 describe('configSchema ↔ expression-ledger reconciliation (#4027)', () => {
   it('every xExpression property a builtin declares is in the ledger', () => {
-    const declared = declaredFromDescriptors();
+    const declared = declaredEverywhere();
     // Sanity: if this ever empties, the derivation broke and the whole ratchet
     // would pass vacuously — the failure mode a ledger test must not have.
     expect(declared.length, 'no xExpression properties found — derivation is broken').toBeGreaterThan(0);
@@ -129,13 +168,39 @@ describe('configSchema ↔ expression-ledger reconciliation (#4027)', () => {
     ).toEqual([]);
   });
 
+  // Each channel must be non-empty on its own. Merging them into one list would
+  // let a broken derivation on either side hide behind the other's results —
+  // which is exactly how the schemaless channel went unnoticed until #4439.
+  it.each([
+    ['descriptor configSchema', declaredFromDescriptors],
+    ['schemaless-node-config.zod.ts', declaredFromSchemalessConfigs],
+  ] as const)('derives at least one slot from the %s channel', (_channel, derive) => {
+    expect(derive().length).toBeGreaterThan(0);
+  });
+
   it('the ledger carries no path a builtin no longer declares', () => {
-    const declared = new Set(declaredFromDescriptors().map(key));
+    const declared = new Set(declaredEverywhere().map(key));
     // Structural predicate surfaces (`config.condition`, `edge.condition`) are
-    // not descriptor properties and are deliberately absent from the ledger, so
-    // every ledger entry must correspond to a real declared property.
+    // not declared config properties on either channel and are deliberately
+    // absent from the ledger, so every ledger entry must correspond to a real
+    // declared property.
     const stale = FLOW_NODE_EXPRESSION_PATHS.map(key).filter((k) => !declared.has(k));
-    expect(stale, 'stale ledger entries — the descriptor no longer declares these').toEqual([]);
+    expect(stale, 'stale ledger entries — no descriptor or schemaless schema declares these').toEqual([]);
+  });
+
+  it('decision.conditions[].expression is covered — the #4439 hole', () => {
+    const decision = FLOW_NODE_EXPRESSION_PATHS.find(
+      (e) => e.nodeType === 'decision' && e.path === 'conditions[].expression',
+    );
+    expect(
+      decision,
+      'the slot a schemaless node could not own: declared bare CEL, walked by neither validator',
+    ).toBeDefined();
+    expect(decision!.role).toBe('predicate');
+    // And it must reach the ledger through the schemaless channel specifically —
+    // `decision` publishes no descriptor configSchema, by design.
+    expect(declaredFromSchemalessConfigs().map(key)).toContain(key(decision!));
+    expect(declaredFromDescriptors().map(key)).not.toContain(key(decision!));
   });
 
   it('screen.fields[].visibleWhen is covered — the #3528 regression', () => {
@@ -185,7 +250,25 @@ describe('resolveFlowNodeExpressions — path resolution (#4027)', () => {
     expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]);
   });
 
+  it('resolves each decision branch predicate, with its index (#4439)', () => {
+    const found = resolveFlowNodeExpressions('decision', {
+      conditions: [
+        { label: 'Yes', expression: "lead.status == 'converted'" },
+        { label: 'No', expression: 'true' },
+      ],
+    });
+    expect(found.map((f) => f.path)).toEqual([
+      'conditions[0].expression',
+      'conditions[1].expression',
+    ]);
+    expect(found.every((f) => f.entry.role === 'predicate')).toBe(true);
+  });
+
   it('returns nothing for a node type with no declared slots', () => {
+    // `config.condition` is a STRUCTURAL surface both validators already walk,
+    // deliberately not a ledger entry — and `assignment` declares no slots.
+    expect(resolveFlowNodeExpressions('assignment', { condition: 'a == b' })).toEqual([]);
+    // A decision branching purely on its edges declares no predicate here.
     expect(resolveFlowNodeExpressions('decision', { condition: 'a == b' })).toEqual([]);
   });
 });