fix(automation): put a screen field's visibleWhen on the wire (#3528)

.changeset/screen-field-visible-when-on-the-wire.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/spec": minor
+"@objectstack/service-automation": minor
+---
+
+fix(automation): a screen field's `visibleWhen` reaches the client (#3528)
+
+`visibleWhen` has been on the `screen` node's designer form since #3304 —
+declared as an expression (`xExpression`), documented as bare CEL, offered to
+authors in Studio. The executor never put it on the wire. `ScreenFieldSpec`
+carried `name` / `label` / `type` / `required` / `options` / `defaultValue` /
+`placeholder` and nothing else, so no client could honour a predicate it never
+received. Authors wrote conditional visibility; every field rendered
+unconditionally; nothing errored.
+
+That is worse than a cosmetic miss, because `required` **is** honoured. A field
+that is optional-by-design but required *when shown* becomes permanently
+required once its predicate is dropped — and a runner that validates the full
+field list then blocks Submit on input the user was never asked for. No resume
+request is issued and the run sits paused forever. HotCRM's lead-conversion
+screen is exactly that shape:
+
+```ts
+{ name: 'createOpportunity', type: 'boolean', required: true },
+{ name: 'opportunityName',   type: 'text', required: true,
+  visibleWhen: 'createOpportunity == true' },
+```
+
+Leave the checkbox unticked and `opportunityName` — which should not be on
+screen at all — blocks the whole conversion.
+
+- `ScreenFieldSpec.visibleWhen` is now part of the contract, documented as
+  client-evaluated bare CEL over the screen's own field names, with the
+  `required`-must-follow-visibility rule stated where implementors will read it.
+- The `screen` executor forwards it **raw**, deliberately uninterpolated: the
+  predicate is re-evaluated per keystroke against values only the client has, so
+  resolving it server-side against flow variables would freeze the field.
+- Covered by tests — the screen wire payload had none for this key.
+
+Clients must evaluate the predicate and skip hidden fields when enforcing
+`required`. Honouring one without the other reproduces the dead-end above.

content/docs/automation/flows.mdx

@@ -210,9 +210,50 @@ rather than evaluating an arbitrary JavaScript string:
 }
 ```
 
+**Screen (flat fields):**
+
+The default shape. Each field is collected as a **bare flow variable**, so a
+later node reads `{subject}`, not `{screen_1.subject}`.
+
+```typescript
+{
+  id: 'screen_1',
+  type: 'screen',
+  label: 'Conversion Details',
+  config: {
+    fields: [
+      { name: 'createOpportunity', label: 'Create Opportunity?', type: 'boolean',
+        required: true, defaultValue: false },
+      { name: 'opportunityName', label: 'Opportunity Name', type: 'text',
+        required: true,
+        visibleWhen: 'createOpportunity == true' },
+    ],
+  },
+}
+```
+
+`visibleWhen` is **bare CEL over the screen's own field names** — not the `{var}`
+template dialect the rest of a node's `config` uses. The client re-evaluates it
+as the user types, against the values collected so far, which is why it cannot
+be a server-interpolated template. Omit it for a field that is always visible.
+
+A hidden field is not collected, and is **not enforced as `required`** — so
+`required` on a `visibleWhen` field means "required *when shown*". That is what
+lets the example above ask for an opportunity name only when one is being
+created.
+
+Two things worth knowing:
+
+- **Give a `required` boolean a `defaultValue`.** An untouched checkbox holds no
+  value at all, which counts as unanswered — without `defaultValue: false` the
+  user cannot express "no" by leaving it clear.
+- **A broken predicate fails open** (the field stays visible) rather than hiding
+  an input the flow may be waiting on. Nothing validates the expression at
+  author time yet, so a typo shows up as a field that never hides.
+
 **Screen (object form):**
 
-A `screen` node normally renders a flat `fields` list. Set `config.objectName`
+Set `config.objectName`
 to instead render an object's **entire** create/edit form — including any
 master-detail child grids (`inlineEdit`) — as one wizard step. The client
 renders the object form and, on save, **persists the record itself** (parent +

packages/services/service-automation/src/builtin/screen-nodes.test.ts

@@ -151,3 +151,89 @@ it('resolves config.functionName as an alias for function (#1870 DX)', async ()
         expect(seen).toEqual([{ cat: 'billing', conf: 0.9 }]);
     });
 });
+
+/** A one-`screen`-node flow whose screen node carries `config`. */
+function screenFlow(config: Record<string, unknown>) {
+    return {
+        name: 'screen_flow',
+        label: 'Screen Flow',
+        type: 'screen' as const,
+        nodes: [
+            { id: 'start', type: 'start' as const, label: 'Start' },
+            { id: 'collect', type: 'screen' as const, label: 'Collect', config },
+            { id: 'end', type: 'end' as const, label: 'End' },
+        ],
+        edges: [
+            { id: 'e1', source: 'start', target: 'collect' },
+            { id: 'e2', source: 'collect', target: 'end' },
+        ],
+    };
+}
+
+describe('screen node — the field wire payload (#3528)', () => {
+    let engine: AutomationEngine;
+
+    beforeEach(() => {
+        engine = new AutomationEngine(createTestLogger());
+        registerScreenNodes(engine, createCtx());
+    });
+
+    /**
+     * `visibleWhen` has been on the screen node's designer form since #3304 but
+     * was dropped when the executor built the paused payload, so it reached no
+     * client and nothing honoured it. HotCRM's lead-conversion screen is the
+     * shape that made it fatal: an optional-by-design field that is `required`
+     * *when shown*. Rendered unconditionally, it blocks Submit on input the
+     * user was never asked for, and the run never resumes.
+     */
+    it('forwards visibleWhen to the paused screen so the client can honour it', async () => {
+        engine.registerFlow('screen_flow', screenFlow({
+            title: 'Conversion Details',
+            fields: [
+                { name: 'createOpportunity', label: 'Create Opportunity?', type: 'boolean', required: true },
+                { name: 'opportunityName', label: 'Opportunity Name', type: 'text', required: true, visibleWhen: 'createOpportunity == true' },
+                { name: 'opportunityAmount', label: 'Opportunity Amount', type: 'currency', visibleWhen: 'createOpportunity == true' },
+            ],
+        }) as any);
+
+        const paused = await engine.execute('screen_flow');
+        expect(paused.status).toBe('paused');
+        const fields = paused.screen!.fields;
+        expect(fields.map((f) => f.visibleWhen)).toEqual([
+            undefined,
+            'createOpportunity == true',
+            'createOpportunity == true',
+        ]);
+        // The conditional field keeps `required` — it is required *when shown*.
+        // Honouring one without the other is what dead-ends the run.
+        expect(fields[1]).toMatchObject({ name: 'opportunityName', required: true });
+    });
+
+    it('leaves visibleWhen undefined when the author declared none', async () => {
+        engine.registerFlow('screen_flow', screenFlow({
+            fields: [{ name: 'subject', label: 'Subject', type: 'text', required: true }],
+        }) as any);
+
+        const paused = await engine.execute('screen_flow');
+        expect(paused.screen!.fields[0].visibleWhen).toBeUndefined();
+    });
+
+    /**
+     * The predicate is re-evaluated by the client on every keystroke against
+     * the values collected so far, which the server cannot see. Interpolating
+     * it here would bake in a verdict from flow variables and freeze the field.
+     */
+    it('forwards the predicate RAW — it is not interpolated against flow variables', async () => {
+        engine.registerFlow('screen_flow', screenFlow({
+            fields: [
+                { name: 'tier', label: 'Tier', type: 'text' },
+                // `{recordId}` is a live flow variable; were the predicate
+                // interpolated it would come back with the value substituted.
+                { name: 'note', label: 'Note', type: 'text', visibleWhen: 'tier == "gold" && "{recordId}" != ""' },
+            ],
+        }) as any);
+
+        const paused = await engine.execute('screen_flow', { params: { recordId: 'lead_1' } } as any);
+        expect(paused.screen!.fields[1].visibleWhen).toBe('tier == "gold" && "{recordId}" != ""');
+    });
+});

packages/services/service-automation/src/builtin/screen-nodes.ts

@@ -137,6 +137,20 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext
           options: Array.isArray(f.options) ? (f.options as Array<{ value: unknown; label: string }>) : undefined,
           defaultValue: f.defaultValue !== undefined ? interpolate(f.defaultValue, variables, context) : undefined,
           placeholder: f.placeholder != null ? String(f.placeholder) : undefined,
+          // Forwarded RAW — deliberately not interpolated. `visibleWhen` is a
+          // predicate the client re-evaluates on every keystroke against the
+          // values collected SO FAR; the server has no view of those, so
+          // resolving it here (to a constant, against flow variables) would
+          // freeze the field visible-or-hidden for the life of the screen.
+          //
+          // It was declared on the designer form from the start but never put
+          // on the wire, so no client could honour it: authors wrote a
+          // predicate, Studio offered the input, and the field rendered
+          // unconditionally. Where that field was also `required`, a runner
+          // validating the full list blocked Submit on a field the user was
+          // never shown — the run then sat paused with no resume request ever
+          // issued (#3528).
+          visibleWhen: f.visibleWhen != null ? String(f.visibleWhen) : undefined,
         })).filter((f) => f.name.length > 0);
         return {
           success: true,

packages/spec/src/contracts/automation-service.ts

@@ -95,6 +95,21 @@ export interface ScreenFieldSpec {
     options?: Array<{ value: unknown; label: string }>;
     defaultValue?: unknown;
     placeholder?: string;
+    /**
+     * Conditional-visibility predicate (ADR-0089's canonical key), evaluated by
+     * the CLIENT against the screen's live collected values — not by the server,
+     * which has no view of what the user has typed so far.
+     *
+     * Bare CEL over the screen's own field names, e.g. `createOpportunity ==
+     * true` shows the field only once that checkbox is ticked. Omit = always
+     * visible.
+     *
+     * A hidden field is not collected, so it must not be enforced as `required`
+     * either — a client that renders this predicate but validates over the full
+     * field list dead-ends the run: Submit blocks on a field the user was never
+     * shown, and no resume is ever issued (#3528).
+     */
+    visibleWhen?: string;
 }
 
 /**