Skip to content

Commit 050cd82

Browse files
os-zhuangclaude
andauthored
feat(spec,service-automation): a flow variable can declare a defaultValue, so "declared" means "bound" (#4697) (#6998)
* feat(spec,service-automation): a flow variable can declare a `defaultValue` (#4697) `FlowVariableSchema` gains an optional `defaultValue`, and the engine binds it whenever no parameter supplies one — so a declared variable is bound on every path and "declared" finally means "bound". Before this, the engine bound an `isInput` variable only when the caller supplied it (`params[name] !== undefined`), so every path that omitted the parameter left the name unbound. Conditions are strict CEL, where an unbound name does not read as `false` — it aborts the predicate and stops the run (hotcrm#643: a screen collecting an optional checkbox, whose untouched path aborted the outgoing edge and persisted nothing). Both run entry points (`execute` and `executeWithoutRetry`) now seed from one shared `seedDeclaredVariables` site, so the retry path behaves identically. Additive: a declaration without `defaultValue` behaves exactly as before. Also lands the docs half of the same gap — under strict CEL only the `vars.`-scoped `has(vars.X)` tests bindedness; the bare `has(X.f)` aborts on the very case it is written for. That truth table is measured against the live evaluator in a test rather than asserted. * docs(spec): regenerate the reference pages for FlowVariable.defaultValue (#4697) Generator output only (`gen:schema && gen:docs`), never hand-edited: the new key's row on the FlowVariable table, and the inline `variables` summary on the two Flow tables gaining its `…` truncation marker now that the shape has a fifth key. `check:docs` reports `230 generated files in sync with packages/spec`. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 45e711a commit 050cd82

9 files changed

Lines changed: 573 additions & 22 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-automation": minor
4+
---
5+
6+
feat(spec,service-automation): a flow variable can declare a `defaultValue`, so "declared" means "bound" (#4697)
7+
8+
Declaring a flow variable used to guarantee nothing at run time. The engine bound
9+
an `isInput` variable **only** when the caller actually supplied it
10+
(`params[name] !== undefined`), so every path that omitted the parameter left the
11+
name unbound — and a flow condition is strict CEL, where an unbound name does not
12+
read as `false`, it **aborts the predicate and stops the run**. The declaration was
13+
documentation, not a guarantee, and there was no metadata form that said "this
14+
variable always has a value".
15+
16+
`FlowVariableSchema` now takes an optional `defaultValue`, and the engine binds it
17+
whenever no parameter supplies one:
18+
19+
```typescript
20+
variables: [
21+
{ name: 'createOpportunity', type: 'boolean', isInput: true, defaultValue: false },
22+
]
23+
```
24+
25+
The rules:
26+
27+
- **A supplied parameter always wins**, including a falsy one — the boundary is
28+
`!== undefined`, so `false`, `null`, `0` and `''` are answers rather than
29+
absences, and only a genuinely missing parameter falls through to the default.
30+
- **A non-input declaration takes its default too.** `isInput: false` means no
31+
parameter can reach the name, so the default is the only thing that can bind it.
32+
- **A declared variable shadows a trigger-record field of the same name**, whether
33+
it was bound from a parameter or from its default — the rule a parameter already
34+
followed. A name cannot resolve out of a different source depending on whether
35+
the caller passed it.
36+
37+
Both run entry points seed from one shared site, so the retry path behaves
38+
identically to the first attempt.
39+
40+
**Additive and opt-in.** A declaration without `defaultValue` behaves exactly as
41+
before, so existing flows parse and run unchanged. The value is not cross-checked
42+
against the declared `type``type` is an open string with no vocabulary to check
43+
against, the same posture as every other `defaultValue` on the authoring surface.
44+
45+
The case this closes came from a screen flow (hotcrm#643): a screen collects an
46+
optional checkbox, the client returns only the fields the user actually touched,
47+
so on the untouched path the variable was never bound, the outgoing edge aborted,
48+
and a lead conversion persisted nothing. The workaround was an `assignment` node
49+
before every screen mirroring the screen field's own `defaultValue`; a declared
50+
default replaces that ceremony.
51+
52+
The docs half of the same gap is now written down too
53+
(`content/docs/automation/flows.mdx`): under strict CEL the guard an author
54+
reaches for first — `has(X.f)`**aborts** on an unbound `X`, the very case it is
55+
written for. Only the `vars.`-scoped `has(vars.X)` tests bindedness. That truth
56+
table is measured against the live evaluator in
57+
`service-automation/src/flow-variable-default.test.ts` rather than asserted, so a
58+
prescription nothing executes cannot quietly stop being true. Prefer
59+
`defaultValue` over either guard: a guard encodes "unanswered means no" into the
60+
predicate and leaves the graph defect in place.

content/docs/automation/flows.mdx

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,10 @@ Two things worth knowing:
336336

337337
- **Give a `required` boolean a `defaultValue`.** An untouched checkbox holds no
338338
value at all, which counts as unanswered — without `defaultValue: false` the
339-
user cannot express "no" by leaving it clear.
339+
user cannot express "no" by leaving it clear. Mirror it on the **flow
340+
variable** as well ([`defaultValue`](#defaultvalue--declaring-a-variable-is-not-the-same-as-binding-it)):
341+
the client returns only the fields the user touched, so on the untouched path
342+
the name is never bound and a downstream condition reading it aborts the run.
340343
- **A broken predicate fails open** (the field stays visible) rather than hiding
341344
an input the flow may be waiting on. `registerFlow()` does check the predicate
342345
as bare CEL — a `{var}` template or a syntax error is a loud registration
@@ -951,6 +954,7 @@ variables: [
951954
{ name: 'input_id', type: 'text', isInput: true, isOutput: false },
952955
{ name: 'result', type: 'object', isInput: false, isOutput: true },
953956
{ name: 'counter', type: 'number', isInput: false, isOutput: false },
957+
{ name: 'createOpportunity', type: 'boolean', isInput: true, defaultValue: false },
954958
]
955959
```
956960

@@ -960,6 +964,96 @@ variables: [
960964
| `type` | `string` | `'text'`, `'number'`, `'boolean'`, `'object'`, `'list'` |
961965
| `isInput` | `boolean` | Available as input parameter |
962966
| `isOutput` | `boolean` | Available as output parameter |
967+
| `defaultValue` | `unknown` | Bound at run start when no parameter supplies one — this is what makes the declaration a guarantee. Optional |
968+
969+
### `defaultValue` — declaring a variable is not the same as binding it
970+
971+
Without `defaultValue`, a declaration is documentation. The engine binds an
972+
`isInput` variable **only** when the caller actually passed it
973+
(`params[name] !== undefined`), so every path that omits the parameter leaves
974+
the name unbound — and an unbound name in a condition does not read as `false`,
975+
it **aborts the predicate and stops the run**.
976+
977+
Declare the default and the name is bound on every path:
978+
979+
{/* os:check */}
980+
```typescript
981+
import { defineFlow } from '@objectstack/spec/automation';
982+
983+
export const LeadConversion = defineFlow({
984+
name: 'lead_conversion',
985+
label: 'Lead Conversion',
986+
type: 'screen',
987+
variables: [
988+
// Bound to `false` on every path the caller leaves it out of.
989+
{ name: 'createOpportunity', type: 'boolean', isInput: true, defaultValue: false },
990+
],
991+
nodes: [
992+
{ id: 'start', type: 'start', label: 'Start' },
993+
{ id: 'end', type: 'end', label: 'End' },
994+
],
995+
edges: [{ id: 'e1', source: 'start', target: 'end' }],
996+
});
997+
```
998+
999+
The rules, all three worth knowing:
1000+
1001+
- **A supplied parameter always wins** — including a falsy one. The boundary is
1002+
`!== undefined`, so `false`, `null`, `0` and `''` are answers, not absences,
1003+
and only a genuinely missing parameter falls through to the default.
1004+
- **A non-input variable takes its default too.** `isInput: false` means no
1005+
parameter can reach the name, so the default is the only thing that can bind
1006+
it.
1007+
- **A declared variable shadows a trigger-record field of the same name**
1008+
whether it was bound from a parameter or from its default. That is the rule a
1009+
parameter already followed; extending it to defaults is what keeps a name from
1010+
resolving out of a different source depending on whether the caller passed it.
1011+
1012+
The case this exists for is a screen that collects something optional. The
1013+
client returns only the fields the user actually touched, so an untouched
1014+
checkbox arrives as nothing at all — and the edge reading it aborts. Give the
1015+
variable a `defaultValue` mirroring the screen field's own, and the untouched
1016+
path routes:
1017+
1018+
```typescript
1019+
variables: [
1020+
{ name: 'createOpportunity', type: 'boolean', isInput: true, defaultValue: false },
1021+
],
1022+
nodes: [
1023+
{ id: 'collect', type: 'screen', label: 'Conversion Details', config: {
1024+
fields: [{ name: 'createOpportunity', label: 'Create Opportunity?',
1025+
type: 'boolean', defaultValue: false }],
1026+
} },
1027+
],
1028+
edges: [
1029+
{ id: 'e_no', source: 'collect', target: 'skip', condition: 'createOpportunity == false' },
1030+
]
1031+
```
1032+
1033+
The value is **not** checked against the declared `type``type` is an open
1034+
string, so there is no vocabulary to check against. It behaves like every other
1035+
`defaultValue` on the authoring surface.
1036+
1037+
<Callout type="warn">
1038+
**Reading a variable that might be unbound: only the `vars.`-scoped guard
1039+
works.** Conditions are strict CEL, and `has()` is not the escape hatch it looks
1040+
like — measured on the live evaluator (and pinned as a test):
1041+
1042+
| expression | `X` unbound | `X = null` | `X = {}` | `X = {f: 1}` |
1043+
| :--- | :--- | :--- | :--- | :--- |
1044+
| `X.f == 1` | **aborts** `Unknown variable: X` | **aborts** `No such key: f` | **aborts** | `true` |
1045+
| `has(X.f)` | **aborts** `Unknown variable: X` | `false` | `false` | `true` |
1046+
| `has(vars.X)` | `false` | `true` | `true` | `true` |
1047+
1048+
So the natural spelling `has(X.f)` fails on exactly the case it was written for,
1049+
and `has(vars.X) && X.f == 1` is the guard that holds (CEL short-circuits, so
1050+
the right-hand read never runs on the unbound leg).
1051+
1052+
Reach for `defaultValue` first anyway. A guard says "unanswered means no" inside
1053+
the predicate and leaves the run's shape depending on what the client happened
1054+
to send; a default removes the unbound state, and every later reader of the flow
1055+
can see it in the declaration.
1056+
</Callout>
9631057

9641058
## Error Handling
9651059

@@ -1152,7 +1246,10 @@ curl -b cookies.txt -X POST \
11521246
**Passing inputs.** Declare variables with `isInput: true`, then send them in
11531247
`params` under the same names — only declared inputs are bound. `recordId` and
11541248
`objectName` are lifted into `params` for you (plus an `<objectName>Id` alias),
1155-
and `event` defaults to `'manual'`.
1249+
and `event` defaults to `'manual'`. A param you *omit* leaves the variable
1250+
unbound unless its declaration carries a
1251+
[`defaultValue`](#defaultvalue--declaring-a-variable-is-not-the-same-as-binding-it);
1252+
a param you send explicitly always wins, `false` and `null` included.
11561253

11571254
**Identity.** The caller's identity (user, positions, permissions, tenant) is
11581255
forwarded into the run, so a `runAs: 'user'` flow executes under that caller's

content/docs/references/api/automation-api.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ const result = AutomationApiErrorCode.parse(data);
9494
| **status** | `Enum<'draft' \| 'active' \| 'obsolete' \| 'invalid'>` | optional | Deployment status |
9595
| **template** | `never` | optional | [REMOVED] `flow.template` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — no designer or engine path ever read it, so flagging a flow as a template/subflow did nothing. Delete the key. Shared logic is invoked via a subflow NODE referencing the flow by name. |
9696
| **type** | `Enum<'autolaunched' \| 'record_change' \| 'schedule' \| 'screen' \| 'api'>` || Flow type |
97-
| **variables** | `{ name: string; type: string; isInput?: boolean; isOutput?: boolean }[]` | optional | Flow variables |
97+
| **variables** | `{ name: string; type: string; isInput?: boolean; isOutput?: boolean; … }[]` | optional | Flow variables |
9898
| **nodes** | `{ id: string; type: string; label: string; config?: Record<string, any>; … }[]` || Flow nodes |
9999
| **edges** | `{ id: string; source: string; target: string; condition?: string \| object; … }[]` || Flow connections |
100100
| **active** | `never` | optional | [REMOVED] `flow.active` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it never had an effect: the engine arms flows from `status`, and `active: false` did NOT stop a flow (worse, the default read as disabled while the engine treated unset as enabled). Delete the key. Use `status: 'obsolete'` (or 'invalid') to unbind and disable a flow, `status: 'active'` to arm it. |

content/docs/references/automation/flow.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ const result = FlowSchema.parse(data);
4848
| **status** | `Enum<'draft' \| 'active' \| 'obsolete' \| 'invalid'>` | optional | Deployment status |
4949
| **template** | `never` | optional | [REMOVED] `flow.template` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — no designer or engine path ever read it, so flagging a flow as a template/subflow did nothing. Delete the key. Shared logic is invoked via a subflow NODE referencing the flow by name. |
5050
| **type** | `Enum<'autolaunched' \| 'record_change' \| 'schedule' \| 'screen' \| 'api'>` || Flow type |
51-
| **variables** | `{ name: string; type: string; isInput?: boolean; isOutput?: boolean }[]` | optional | Flow variables |
51+
| **variables** | `{ name: string; type: string; isInput?: boolean; isOutput?: boolean; … }[]` | optional | Flow variables |
5252
| **nodes** | `{ id: string; type: string; label: string; config?: Record<string, any>; … }[]` || Flow nodes |
5353
| **edges** | `{ id: string; source: string; target: string; condition?: string \| object; … }[]` || Flow connections |
5454
| **active** | `never` | optional | [REMOVED] `flow.active` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it never had an effect: the engine arms flows from `status`, and `active: false` did NOT stop a flow (worse, the default read as disabled while the engine treated unset as enabled). Delete the key. Use `status: 'obsolete'` (or 'invalid') to unbind and disable a flow, `status: 'active'` to arm it. |
@@ -142,6 +142,7 @@ const result = FlowSchema.parse(data);
142142
| **type** | `string` || Data type (text, number, boolean, object, list) |
143143
| **isInput** | `boolean` || Is input parameter |
144144
| **isOutput** | `boolean` || Is output parameter |
145+
| **defaultValue** | `any` | optional | Value bound at run start when no parameter supplies one — this is what makes a declared variable always bound. An explicitly supplied param wins, including `false` and `null`; the boundary is `params[name] !== undefined`. |
145146

146147

147148
---

packages/services/service-automation/src/engine.ts

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2762,14 +2762,7 @@ export class AutomationEngine implements IAutomationService {
27622762
if (reentryKey) this.activeRecordFlows.add(reentryKey);
27632763

27642764
// Initialize variable context
2765-
const variables = new Map<string, unknown>();
2766-
if (flow.variables) {
2767-
for (const v of flow.variables) {
2768-
if (v.isInput && context?.params?.[v.name] !== undefined) {
2769-
variables.set(v.name, context.params[v.name]);
2770-
}
2771-
}
2772-
}
2765+
const variables = this.seedDeclaredVariables(flow, context);
27732766
// Inject trigger record. `$record` is the canonical handle; `record` is a
27742767
// friendlier alias so templates/conditions can write `{record.title}` and
27752768
// `record.status`. We also flatten the record's own fields to top-level
@@ -5707,6 +5700,56 @@ export class AutomationEngine implements IAutomationService {
57075700
return { success: false, error: lastError, durationMs: Date.now() - startTime };
57085701
}
57095702

5703+
/**
5704+
* Seed a run's variable map from the flow's DECLARED variables — the one
5705+
* place `declared` is turned into `bound` (#4697).
5706+
*
5707+
* Two sources, in this precedence:
5708+
*
5709+
* 1. `context.params[name]`, for an `isInput` variable, when the caller
5710+
* supplied it. The boundary is `!== undefined`, so an explicit `false`,
5711+
* `null`, `0` or `''` is a supplied value and wins over the default —
5712+
* only *absence* falls through.
5713+
* 2. `defaultValue`, when the declaration carries one. This is the half
5714+
* that did not exist before #4697: a declared variable the caller left
5715+
* out stayed **unbound**, and conditions are strict CEL, where reading
5716+
* an unbound name ABORTS the predicate (`Unknown variable: X`) instead
5717+
* of yielding `false`. A screen flow collecting an optional checkbox hit
5718+
* exactly that — the runner returns only the fields the user touched, so
5719+
* the untouched path aborted the outgoing edge and the run stopped
5720+
* (hotcrm#643). The workaround was an `assignment` node per screen,
5721+
* mirroring the screen field's own `defaultValue`.
5722+
*
5723+
* `defaultValue` is honoured for a NON-input variable too: params are not
5724+
* readable there by definition, so the default is the only thing that can
5725+
* bind it, and "declared means bound" would otherwise hold for half the
5726+
* declarations. A declaration with no `defaultValue` behaves exactly as
5727+
* before — existing flows are untouched.
5728+
*
5729+
* Seeding happens BEFORE the trigger record is flattened to top-level
5730+
* names, and that flattening skips names already present. So a declared
5731+
* variable — bound from a param or from its default — shadows a record
5732+
* field of the same name, which is the rule params already followed; a
5733+
* default cannot make the same name resolve from a different source
5734+
* depending on whether the caller passed it.
5735+
*/
5736+
private seedDeclaredVariables(
5737+
flow: FlowParsed,
5738+
context?: AutomationContext,
5739+
): Map<string, unknown> {
5740+
const variables = new Map<string, unknown>();
5741+
if (!flow.variables) return variables;
5742+
for (const v of flow.variables) {
5743+
const supplied = v.isInput ? context?.params?.[v.name] : undefined;
5744+
if (supplied !== undefined) {
5745+
variables.set(v.name, supplied);
5746+
} else if (v.defaultValue !== undefined) {
5747+
variables.set(v.name, v.defaultValue);
5748+
}
5749+
}
5750+
return variables;
5751+
}
5752+
57105753
/**
57115754
* Execute a flow without triggering retry logic (used by retryExecution to prevent recursion).
57125755
*/
@@ -5724,14 +5767,7 @@ export class AutomationEngine implements IAutomationService {
57245767
return { success: false, error: `Flow '${flowName}' is disabled` };
57255768
}
57265769

5727-
const variables = new Map<string, unknown>();
5728-
if (flow.variables) {
5729-
for (const v of flow.variables) {
5730-
if (v.isInput && context?.params?.[v.name] !== undefined) {
5731-
variables.set(v.name, context.params[v.name]);
5732-
}
5733-
}
5734-
}
5770+
const variables = this.seedDeclaredVariables(flow, context);
57355771
if (context?.record) {
57365772
variables.set('$record', context.record);
57375773
}

0 commit comments

Comments
 (0)