From 5cc47d1d7ff23763a2eae743c53b72bc3da8e77e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 03:59:00 +0000 Subject: [PATCH 1/2] fix(automation): put a screen field's `visibleWhen` on the wire (#3528) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `visibleWhen` has been on the `screen` node's designer form since #3304 — declared `xExpression`, documented as bare CEL, offered in Studio — but the executor dropped it when building the paused payload. `ScreenFieldSpec` stopped at `placeholder`, so no client ever received the predicate and none could honour it. Authors wrote conditional visibility; every field rendered unconditionally; nothing errored. `required` *is* honoured, which is what makes this fatal rather than cosmetic. A field that is optional-by-design but required when shown becomes permanently required once its predicate is dropped, and a runner validating the full field list then blocks Submit on input the user was never asked for — no resume request is issued and the run stays paused. That is the reported #3528 symptom, reproduced in a browser against HotCRM's lead-conversion screen on both the console shipped with 16.1.0 and current objectui main: trigger fires, the screen renders all three fields, Submit issues zero network requests. - `ScreenFieldSpec.visibleWhen` joins the contract, documented as client-evaluated bare CEL over the screen's own field names, and states the rule an implementor needs: a hidden field must not be enforced as required. - The executor forwards it raw. Interpolating here would resolve it once against flow variables, freezing a predicate the client must re-evaluate against values only it can see. - Tests for the screen wire payload, which had none for this key; both new assertions verified to fail without the forwarding line. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0122xvfanLmniNbsAPtg5hk3 --- .../screen-field-visible-when-on-the-wire.md | 41 +++++++++ .../src/builtin/screen-nodes.test.ts | 86 +++++++++++++++++++ .../src/builtin/screen-nodes.ts | 14 +++ .../spec/src/contracts/automation-service.ts | 15 ++++ 4 files changed, 156 insertions(+) create mode 100644 .changeset/screen-field-visible-when-on-the-wire.md diff --git a/.changeset/screen-field-visible-when-on-the-wire.md b/.changeset/screen-field-visible-when-on-the-wire.md new file mode 100644 index 0000000000..89b257430b --- /dev/null +++ b/.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. diff --git a/packages/services/service-automation/src/builtin/screen-nodes.test.ts b/packages/services/service-automation/src/builtin/screen-nodes.test.ts index 2a6566e3be..6be603fb22 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.test.ts +++ b/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) { + 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}" != ""'); + }); +}); diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index 65c4a6ee92..cb9942240d 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/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, diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index 9a93193ce0..35c09ae00d 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/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; } /** From e1dd5dfa2787f5feeda3ba9c434057799bbadeea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 04:40:05 +0000 Subject: [PATCH 2/2] docs(automation): document the flat screen-field shape and `visibleWhen` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flows guide covered the `object-form` screen step but never the default flat `fields` list, so the shape most screens actually use was undocumented — including `visibleWhen`, which this branch just made live on the wire. Records the three things an author cannot infer from the type: the predicate is bare CEL over the screen's own field names (NOT the `{var}` template dialect the rest of a node's config uses), `required` on a conditional field means "required when shown", and a `required` boolean needs `defaultValue: false` or the user cannot express "no" by leaving the box clear. Also notes that a broken predicate fails open, since nothing validates it at author time yet. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0122xvfanLmniNbsAPtg5hk3 --- content/docs/automation/flows.mdx | 43 ++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index d64ec004bb..e91ce51449 100644 --- a/content/docs/automation/flows.mdx +++ b/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 +