diff --git a/.changeset/19961-decision-branch-expression-absent-refused.md b/.changeset/19961-decision-branch-expression-absent-refused.md new file mode 100644 index 00000000000..979bb2e788a --- /dev/null +++ b/.changeset/19961-decision-branch-expression-absent-refused.md @@ -0,0 +1,61 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': minor +--- + +fix(spec)!: a `decision` branch with no `expression` — the key absent, or `null` — is refused at authoring (#19961) + +Clause-②: no (narrowing) + + + +**BREAKING** — an accept-set narrowing on one authored flow-node slot, shipped as +`minor` under the launch-window convention (`check-changeset-no-major` refuses +`major` until GA; breaking-ness is carried by this banner and the ADR-0087 +disposition above, not by the level). + +**What changed.** `DecisionConditionSchema` declares a branch `{ label, expression }` +with `expression` a required `z.string()`. Nothing enforced that: a decision node's +`config` is an open record no schema is parsed against, and the expression ledger's +resolver skipped an absent value as "not authored". So `conditions: [{ label: 'y' }]` +passed `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate`, +and the run then failed at that branch — the executor evaluates every branch it +reaches, and a branch with no `expression` is a condition with no `source`, which +`evaluateCondition` refuses. The ledger now marks the slot `required` (reconciled +against the schema's own `required` list), and the branch is refused at all three +doors through the walk and the function that already refuse a blank one — by +`FlowSchema.parse` with a `custom` issue anchored at the slot (for example +`nodes.1.config.conditions.0.expression`), by `registerFlow` and `objectstack validate` +through that same parse, and by `validateStackExpressions` for a stack handed to it +directly — with one message, led by the published `PREDICATE_SLOT_STRING_REFUSAL` +sentence. `expression: null` is refused the same way, and so is a branch that wrote +its predicate under `condition` (the edge's spelling), which has no `expression` +either. The Studio flow designer writes the refused shape when a branch row's +expression cell is left empty. Where such a branch already sits, the whole flow is +refused: registered from the metadata +registry or `sys_metadata` at boot, it is skipped with a `failed to register flow` +warn naming it while the flows beside it register; a `defineStack({ flows })` source +throws `StackSchemaInvalidError` for the whole stack; an artifact file is refused +whole at load. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `conditions: [{ label: 'high' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` | +| `conditions: [{ label: 'high', condition: 'record.amount > 10000' }]` | the same predicate under `expression` | +| `conditions: [{ label: 'high', expression: null }]` | the predicate you meant, or `expression: 'false'` to keep the branch and never take it | + +**One-line fix:** write the predicate under `expression`. `expression: 'false'` keeps +the branch and its label and never takes it — a change of behaviour, not a preserved +one: a run that reached the branch used to FAIL there, and now routes on to the next +branch or the declared fallback. ⚠️ Do not drop a decision's only branch: the node +then routes by its out-edges alone, and the out-edge that branch labelled is no +longer held back. + +**Unchanged.** A branch carrying a non-blank predicate parses, registers and +validates as before; a blank one keeps its refusal and its own prescription +(`flow-predicate-slot-blank-string-refused`); a `decision` with no `conditions`, or +an empty list, still routes by its out-edges; an absent screen field `visibleWhen` +is still legal (that slot is not required); and `PREDICATE_SLOT_STRING_REFUSAL` +keeps its name and its text. diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index bc0c18046fd..bb19b412dca 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -15,6 +15,7 @@ import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, PREDICATE_SLOT_STRING_REFUSAL, STRUCTURAL_CONDITION_SHAPE_REFUSAL, + predicateSlotRefusal, } from '@objectstack/spec/automation'; import { @@ -4398,6 +4399,75 @@ describe('a blank string in a ledger predicate slot (#17493)', () => { }); }); +/** + * [#19961] A `decision` branch with no `expression`, at the THIRD door: + * `objectstack validate`'s expression pass. + * + * `DecisionConditionSchema` declares `expression` `z.string()`, not optional, + * but `conditions: [{ label: 'y' }]` reported NOTHING here: the resolver skipped + * the absent value as "not authored", and `checkDeclaredPredicate` returned + * early on `null` / absent besides. The run then failed at the branch. The + * ledger now marks the slot `required`, the resolver emits the absent value + * there, and this pass refuses it through `predicateSlotRefusal` — the same + * call, the same message, as `FlowSchema.parse` and `registerFlow`. + * + * The table is the one those two doors run (in `spec` and `service-automation`): + * nothing, a blank string, a real predicate — asserted by `where`, severity and + * the full message, read off the spec's own function. + * + * ⚠️ Through the CLI, `objectstack validate` meets the absent value first at + * its schema step (`FlowSchema.parse` refuses it there, with the same message). + * This pass is what answers for a stack handed to `validateStackExpressions` + * directly, and it is what these pins drive. + */ +describe('a decision branch with no `expression` (#19961)', () => { + const flowStack = (...branches: Record[]) => ({ + flows: [{ + name: 'absent_flow', + nodes: [{ id: 'start', type: 'start' }, { id: 'check', type: 'decision', config: { conditions: branches } }], + edges: [], + }], + }); + const errorsOf = (stack: unknown) => + validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error'); + const WHERE_0 = "flow 'absent_flow' · node 'check' (decision) decision branch expression at config.conditions[0].expression"; + + it.each([ + { name: 'no `expression` key — the #19961 shape', branch: { label: 'y' }, refused: true, refusedWith: undefined }, + { name: '`expression: null`', branch: { label: 'y', expression: null }, refused: true, refusedWith: null }, + { name: 'the predicate under the edge\'s spelling `condition`', branch: { label: 'y', condition: 'true' }, refused: true, refusedWith: undefined }, + { name: 'a blank string — the #17493 control', branch: { label: 'y', expression: ' ' }, refused: true, refusedWith: ' ' }, + { name: 'a real predicate — the accept control', branch: { label: 'y', expression: 'true' }, refused: false, refusedWith: undefined }, + ] as Array<{ name: string; branch: Record; refused: boolean; refusedWith: unknown }>)('$name', ({ branch, refused, refusedWith }) => { + const found = errorsOf(flowStack(branch)); + if (!refused) { + expect(found).toHaveLength(0); + return; + } + expect(found).toHaveLength(1); + expect(found[0].severity).toBe('error'); + expect(found[0].where).toBe(WHERE_0); + expect(found[0].message).toBe(predicateSlotRefusal(refusedWith)!.message); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + + it('names WHICH branch: an absent second branch is located at index 1, the valid first one is not', () => { + const found = errorsOf(flowStack({ label: 'a', expression: 'amount > 1' }, { label: 'b' })); + expect(found.map((i) => i.where)).toEqual([ + "flow 'absent_flow' · node 'check' (decision) decision branch expression at config.conditions[1].expression", + ]); + expect(found[0].source).toBe(''); + }); + + it('CONTROL — a decision with no branch, and a screen field with no `visibleWhen`, report nothing', () => { + expect(errorsOf({ flows: [{ name: 'f', nodes: [{ id: 'check', type: 'decision', config: {} }], edges: [] }] })).toHaveLength(0); + expect(errorsOf(flowStack())).toHaveLength(0); + expect(errorsOf({ + flows: [{ name: 'f', nodes: [{ id: 'form', type: 'screen', config: { fields: [{ name: 'amount', type: 'number' }] } }], edges: [] }], + })).toHaveLength(0); + }); +}); + /** * [#20078] A field-level predicate that reads THROUGH a reference field is * refused at authoring, with the repair that is true for the root it reads. diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index a22c05d6759..716388d9629 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -1401,7 +1401,14 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { * a field-existence pass would report every field name as unknown. */ const checkDeclaredPredicate = (where: string, raw: unknown): { refused: boolean } => { - if (raw == null) return { refused: false }; + // [#19961] No `raw == null` early return: whether an absent value is a + // finding is the resolver's call, not this pass's. It emits absent / `null` + // only for a `required` ledger slot (a `decision` branch's `expression`), + // and there it is a refusal — the one `predicateSlotRefusal` gives the + // other two doors. An early return here answered "valid" for the very + // value `FlowSchema.parse` refuses, for any caller of + // `validateStackExpressions` that did not parse first. + // // [#15572] The slot is declared bare CEL TEXT, so a non-string — the // `{ dialect, source }` envelope above all — is refused on SHAPE before // anything tries to read a source out of it. The refusal is the spec's, diff --git a/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts b/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts index 89f17ccf4b6..fa8e9821f0b 100644 --- a/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts +++ b/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts @@ -48,6 +48,8 @@ interface SchemaNode { items?: SchemaNode; /** `true` = an open map with untyped values; an object = the schema every value takes. */ additionalProperties?: boolean | SchemaNode; + /** The keys of `properties` this object requires (JSON Schema `required`). */ + required?: string[]; xExpression?: string; } @@ -83,9 +85,9 @@ const ROLE_BY_MARKER: Record = { function collectExpressionProps( schema: SchemaNode | undefined, prefix = '', -): { path: string; marker: string }[] { +): { path: string; marker: string; required: boolean }[] { if (!schema || typeof schema !== 'object') return []; - const out: { path: string; marker: string }[] = []; + const out: { path: string; marker: string; required: boolean }[] = []; if (schema.properties) { for (const [key, prop] of Object.entries(schema.properties)) { @@ -96,7 +98,11 @@ function collectExpressionProps( const here = prefix ? `${prefix}.${key}${isObjectArray ? '[]' : ''}` : `${key}${isObjectArray ? '[]' : ''}`; - if (typeof prop.xExpression === 'string') out.push({ path: here, marker: prop.xExpression }); + // [#19961] Whether the declaring object REQUIRES the slot — read off the + // same schema the marker is, so the ledger's `required` flag is + // reconciled against the contract rather than restated beside it. + const required = Array.isArray(schema.required) && schema.required.includes(key); + if (typeof prop.xExpression === 'string') out.push({ path: here, marker: prop.xExpression, required }); if (isObjectArray) out.push(...collectExpressionProps(prop.items, here)); else out.push(...collectExpressionProps(prop, here)); } @@ -110,7 +116,8 @@ function collectExpressionProps( const values = schema.additionalProperties; if (values && typeof values === 'object') { const here = prefix ? `${prefix}.*` : '*'; - if (typeof values.xExpression === 'string') out.push({ path: here, marker: values.xExpression }); + // A map value is never "required": the map's keys are the author's own. + if (typeof values.xExpression === 'string') out.push({ path: here, marker: values.xExpression, required: false }); out.push(...collectExpressionProps(values, here)); } return out; @@ -119,7 +126,7 @@ function collectExpressionProps( const engine = new AutomationEngine(silentLogger()); installBuiltinNodes(engine, ctx()); -type DeclaredSlot = { nodeType: string; path: string; role: FlowNodeExpressionRole }; +type DeclaredSlot = { nodeType: string; path: string; role: FlowNodeExpressionRole; required: boolean }; /** Resolve an `xExpression` marker to its ledger role, failing loudly on an unknown one. */ function roleOf(nodeType: string, path: string, marker: string): FlowNodeExpressionRole { @@ -137,8 +144,8 @@ 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)) { - found.push({ nodeType: descriptor.type, path, role: roleOf(descriptor.type, path, marker) }); + for (const { path, marker, required } of collectExpressionProps(schema)) { + found.push({ nodeType: descriptor.type, path, role: roleOf(descriptor.type, path, marker), required }); } } return found; @@ -163,8 +170,8 @@ function declaredFromDescriptors(): DeclaredSlot[] { 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) }); + for (const { path, marker, required } of collectExpressionProps(json as SchemaNode)) { + found.push({ nodeType, path, role: roleOf(nodeType, path, marker), required }); } } return found; @@ -213,6 +220,33 @@ describe('configSchema ↔ expression-ledger reconciliation (#4027)', () => { expect(stale, 'stale ledger entries — no descriptor or schemaless schema declares these').toEqual([]); }); + /** + * [#19961] The ledger's `required` flag is what makes the resolver emit an + * ABSENT value for the doors to refuse — so it must say exactly what the + * declaring channel's `required` list says, in both directions. A flag the + * contract does not back would refuse a legal omission (an absent + * `visibleWhen` shows the field); a requirement the ledger misses is the + * #19961 shape again — declared required, admitted absent at every door. + * + * Reconciled over the `predicate` role, the one role the flag acts on: the + * resolver emits an absent value only for a required PREDICATE slot. The + * channels require two `flow-template` slots too (`loop.collection`, + * `map.collection`), and no door refuses their absence — their executors + * parse their own config — so the flag stays off there, and the second + * assertion pins that it is never set on another role. + */ + it('the ledger marks `required` exactly the predicate slots the declaring channel requires (#19961)', () => { + const declared = declaredEverywhere().filter((d) => d.role === 'predicate'); + const requiredByChannel = declared.filter((d) => d.required).map(key).sort(); + const requiredByLedger = FLOW_NODE_EXPRESSION_PATHS.filter((e) => e.role === 'predicate' && e.required).map(key).sort(); + expect(requiredByLedger, 'ledger `required` flags disagree with the declaring channel').toEqual(requiredByChannel); + expect(FLOW_NODE_EXPRESSION_PATHS.filter((e) => e.role !== 'predicate' && e.required).map(key), '`required` acts on the predicate role only').toEqual([]); + // Non-vacuous: the one required predicate slot there is today is derived, + // not assumed — and the optional one (`visibleWhen`) is derived as optional. + expect(requiredByChannel).toEqual(['decision.conditions[].expression (predicate)']); + expect(declared.filter((d) => !d.required).map(key)).toEqual(['screen.fields[].visibleWhen (predicate)']); + }); + it('decision.conditions[].expression is covered — the #4439 hole', () => { const decision = FLOW_NODE_EXPRESSION_PATHS.find( (e) => e.nodeType === 'decision' && e.path === 'conditions[].expression', diff --git a/packages/services/service-automation/src/decision-branch-expression-absent.test.ts b/packages/services/service-automation/src/decision-branch-expression-absent.test.ts new file mode 100644 index 00000000000..4e325fe5a2b --- /dev/null +++ b/packages/services/service-automation/src/decision-branch-expression-absent.test.ts @@ -0,0 +1,156 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #19961 — a `decision` branch with no `expression` is refused at + * `AutomationEngine.registerFlow`, the second of the three doors. + * + * `DecisionConditionSchema` declares `expression` `z.string()`, not optional, + * yet `conditions: [{ label: 'y' }]` registered clean: nothing parses a + * decision's open `config` against that schema, and the expression ledger's + * resolver skipped the absent value as "not authored". The run then failed at + * the branch — the executor hands `evaluateCondition` a `{ dialect, source }` + * envelope whose `source` is `undefined`, and the shape gate refuses it. The + * build accepted what the run refused; the ledger now marks the slot + * `required`, so the absent value goes through the same walk and the same + * `predicateSlotRefusal` as the blank string (#17493). + * + * ## Which gate answers, measured rather than assumed + * + * `registerFlow` parses first (`canonicalizeStoredFlow` → `FlowSchema.parse`), + * and the flow parse refuses the absent branch predicate itself — so the + * refusal this door hands back is the PARSE's: a Zod issue with code `custom`, + * anchored at the slot, whose message is `predicateSlotRefusal`'s. That is the + * same two-layer shape the blank has (`predicate-slot-blank.test.ts`): the + * engine's own ledger pass would refuse the value through the same call one + * step later. The table below is the one `FlowSchema.parse` runs in `spec` + * (`flow-decision-branch-expression-absent.test.ts`), so the two doors are + * held to the same code, path and message. + */ +import { describe, expect, it, vi } from 'vitest'; +import { PREDICATE_SLOT_STRING_REFUSAL, predicateSlotRefusal } from '@objectstack/spec/automation'; + +import { AutomationEngine } from './engine.js'; +import { registerLogicNodes } from './builtin/logic-nodes.js'; + +const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any; + +type Node = Record; + +function flowWith(...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle, { id: 'end', type: 'end', label: 'End' }]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name: 'absent_flow', label: 'Absent flow', type: 'autolaunched', nodes, edges }; +} + +/** A decision whose branches are written exactly as given — no key is added. */ +const decision = (...branches: Node[]): Node => ({ + id: 'branch', type: 'decision', label: 'Branch', config: { conditions: branches }, +}); + +/** What `registerFlow` threw, or `undefined` when it registered. */ +function refusalOf(engine: AutomationEngine, flow: unknown): { message: string; issues?: Array<{ code: string; path: unknown[]; message: string }> } | undefined { + try { + engine.registerFlow('absent_flow', flow as never); + return undefined; + } catch (e) { + return e as never; + } +} + +const TABLE: Array<{ name: string; branch: Node; refused: boolean; refusedWith?: unknown }> = [ + { name: 'no `expression` key — the #19961 shape', branch: { label: 'y' }, refused: true, refusedWith: undefined }, + { name: '`expression: null`', branch: { label: 'y', expression: null }, refused: true, refusedWith: null }, + { name: 'the predicate under the edge\'s spelling `condition`', branch: { label: 'y', condition: 'true' }, refused: true, refusedWith: undefined }, + { name: 'a blank string — the #17493 control', branch: { label: 'y', expression: ' ' }, refused: true, refusedWith: ' ' }, + { name: 'a real predicate — the accept control', branch: { label: 'y', expression: 'true' }, refused: false }, +]; + +describe('registerFlow refuses a decision branch with no `expression` (#19961)', () => { + it.each(TABLE)('$name', async ({ branch, refused, refusedWith }) => { + const engine = new AutomationEngine(silentLogger); + const refusal = refusalOf(engine, flowWith(decision(branch))); + if (!refused) { + expect(refusal).toBeUndefined(); + expect(await engine.getFlow('absent_flow')).not.toBeNull(); + return; + } + expect(refusal).toBeDefined(); + expect(refusal!.issues?.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'conditions', 0, 'expression']], + ]); + expect(refusal!.issues![0].message).toBe(predicateSlotRefusal(refusedWith)!.message); + expect(refusal!.issues![0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(await engine.getFlow('absent_flow')).toBeNull(); + }); + + it('refuses an absent branch inside an ADR-0031 region body, anchored where the author wrote it', () => { + const refusal = refusalOf(new AutomationEngine(silentLogger), flowWith({ + id: 'sweep', type: 'loop', label: 'Sweep', + config: { collection: '{items}', itemVariable: 'item', body: { nodes: [decision({ label: 'y' })], edges: [] } }, + })); + expect(refusal?.issues?.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'conditions', 0, 'expression']], + ]); + }); + + it('CONTROL — a decision that declares no branch still registers: it routes by its out-edges', () => { + expect(refusalOf(new AutomationEngine(silentLogger), flowWith({ id: 'branch', type: 'decision', label: 'B', config: {} }))).toBeUndefined(); + expect(refusalOf(new AutomationEngine(silentLogger), flowWith(decision()))).toBeUndefined(); + }); +}); + +/** + * What the refused shape DID at run time, measured on the real decision + * executor — the ground the prescription stands on. It can no longer + * register, so the run registers a placeholder and deletes the stored + * branch's `expression` before executing, the way `predicate-slot-blank.test.ts` + * measures the blank. + * + * The blank KEPT a run (it evaluated `false`), so its prescription can offer + * `expression: 'false'` as "keep what ran". The absent predicate did not: the + * run FAILED at the branch. So `'false'` is offered for what it is — keep the + * branch and its label, never take it — and nothing is claimed to be kept. + */ +describe('what a decision branch with no `expression` did at run time (#19961)', () => { + async function runOf(expression: string | undefined, placeholder = 'true') { + const engine = new AutomationEngine(silentLogger); + registerLogicNodes(engine, { logger: silentLogger, getService: () => undefined } as never); + engine.registerNodeExecutor({ type: 'mark', async execute() { return { success: true }; } }); + engine.sealNodeTypeVocabulary(); + const parsed = engine.registerFlow('route', { + name: 'route', label: 'Route', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'd', type: 'decision', label: 'D', config: { conditions: [{ label: 'y', expression: expression ?? placeholder }] } }, + { id: 'x', type: 'mark', label: 'x' }, + { id: 'fallback', type: 'mark', label: 'fallback' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'd' }, + { id: 'e1', source: 'd', target: 'x', label: 'y' }, + { id: 'e2', source: 'd', target: 'fallback', isDefault: true }, + ], + } as never); + if (expression === undefined) { + delete (parsed.nodes[1].config as { conditions: Array> }).conditions[0].expression; + } + const result = await engine.execute('route', { params: {} } as never); + const [log] = await engine.listRuns('route'); + return { + success: result.success, + error: String(result.error ?? ''), + ran: (log?.steps ?? []).filter((s) => s.status === 'success').map((s) => s.nodeId), + }; + } + + it('the absent predicate failed the run at the branch — there was no run to keep', async () => { + const run = await runOf(undefined); + expect(run.success).toBe(false); + expect(run.error).toContain('condition evaluation error'); + expect(run.ran).toEqual(['start']); + }); + + it('`expression: \'false\'` keeps the branch and never takes it — the run goes to the fallback', async () => { + expect(await runOf('false')).toEqual({ success: true, error: '', ran: ['start', 'd', 'fallback'] }); + }); +}); diff --git a/packages/services/service-automation/src/decision-predicate-envelope.test.ts b/packages/services/service-automation/src/decision-predicate-envelope.test.ts index 100e0268012..62188b1f1dc 100644 --- a/packages/services/service-automation/src/decision-predicate-envelope.test.ts +++ b/packages/services/service-automation/src/decision-predicate-envelope.test.ts @@ -121,11 +121,23 @@ describe('decision branch predicate — envelope in a `z.string()` slot (#15572) * string is now refused here too — by `FlowSchema.parse` inside * `registerFlow`, under this slot's own sentence. A non-blank string and an * absent predicate still register, exactly as this test always said. + * + * RE-JUDGED IN PLACE AGAIN (#19961) — the absent half. It pinned + * `decisionFlow('str_absent', undefined)` as registering: "an absent + * predicate still registers". That was "not authored" on the resolver's + * side only. `DecisionConditionSchema` declares `expression` a REQUIRED + * `z.string()`, and the executor evaluates every branch it reaches, so a + * branch with no `expression` failed the run at the branch — the build + * accepted what the run refused. The ledger now marks the slot `required` + * and the absent value is refused through the same `predicateSlotRefusal` + * as the blank, under the same sentence, with its own detail. */ - it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => { + it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493) and the absent one (#19961)', () => { expect(() => engine.registerFlow('str_ok', decisionFlow('str_ok', 'record.rating >= 4'))).not.toThrow(); expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', ' '))).toThrow(PREDICATE_SLOT_STRING_REFUSAL); - expect(() => engine.registerFlow('str_absent', decisionFlow('str_absent', undefined))).not.toThrow(); + const absent = () => engine.registerFlow('str_absent', decisionFlow('str_absent', undefined)); + expect(absent).toThrow(PREDICATE_SLOT_STRING_REFUSAL); + expect(absent).toThrow('Found nothing — the key is absent where the slot is required'); }); /** diff --git a/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts b/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts new file mode 100644 index 00000000000..d268880f293 --- /dev/null +++ b/packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts @@ -0,0 +1,109 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #19961 — a `decision` branch with no `expression` is refused at + * `FlowSchema.parse`, the first of the three doors. + * + * `DecisionConditionSchema` declares `expression` `z.string()`, not optional, + * but nothing parses a node's open `config` against that schema, so + * `conditions: [{ label: 'y' }]` passed `FlowSchema.parse`, + * `AutomationEngine.registerFlow` and `objectstack validate` — and the + * executor then evaluated the branch as a condition with no `source`, which + * `evaluateCondition` refuses, failing the run at the branch. + * + * The refusal rides the walk the blank string already goes through (#17493): + * the expression ledger marks the slot `required`, `resolveFlowNodeExpressions` + * emits the absent value there, and `predicateSlotRefusal` answers it under the + * same lead sentence. So this file is ONE table over the three values the slot + * can hold on a branch that exists — nothing, a blank string, a real predicate + * — and asserts the issue `code`, the `path` and the full message (read off the + * spec's own `predicateSlotRefusal`, never re-spelled) for each. The other two + * doors run the same table in their own packages + * (`service-automation`'s `decision-branch-expression-absent.test.ts`, + * `lint`'s `validate-expressions.test.ts`). + */ + +import { describe, expect, it } from 'vitest'; + +import { PREDICATE_SLOT_STRING_REFUSAL, predicateSlotRefusal } from './flow-node-expression-paths'; +import { FlowSchema } from './flow.zod'; + +type Node = Record; + +/** start → → end, chained by unconditional edges. */ +function flowWith(...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle, { id: 'end', type: 'end', label: 'End' }]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name: 'absent_probe', label: 'Absent probe', type: 'autolaunched', nodes, edges }; +} + +/** A decision whose branches are written exactly as given — no key is added. */ +const decision = (...branches: Node[]): Node => ({ + id: 'branch', type: 'decision', label: 'Branch', config: { conditions: branches }, +}); + +const loopAround = (inner: Node): Node => ({ + id: 'sweep', type: 'loop', label: 'Sweep', + config: { collection: '{items}', itemVariable: 'item', body: { nodes: [inner], edges: [] } }, +}); + +function issuesOf(flow: unknown) { + const result = FlowSchema.safeParse(flow); + return result.success ? [] : result.error.issues; +} + +const AT_BRANCH_0 = ['nodes', 1, 'config', 'conditions', 0, 'expression']; + +/** + * The table: what the branch holds → what this door answers. `refusedWith` is + * the value handed to `predicateSlotRefusal` for the expected message, so each + * refused row's message is the ONE judge's, byte for byte. + */ +const TABLE: Array<{ name: string; branch: Node; refused: boolean; refusedWith?: unknown }> = [ + { name: 'no `expression` key — the #19961 shape', branch: { label: 'y' }, refused: true, refusedWith: undefined }, + { name: '`expression: null`', branch: { label: 'y', expression: null }, refused: true, refusedWith: null }, + { name: 'the predicate under the edge\'s spelling `condition`', branch: { label: 'y', condition: 'true' }, refused: true, refusedWith: undefined }, + { name: 'a blank string — the #17493 control', branch: { label: 'y', expression: ' ' }, refused: true, refusedWith: ' ' }, + { name: 'a real predicate — the accept control', branch: { label: 'y', expression: 'true' }, refused: false }, +]; + +describe('FlowSchema.parse refuses a decision branch with no `expression` (#19961)', () => { + it.each(TABLE)('$name', ({ branch, refused, refusedWith }) => { + const issues = issuesOf(flowWith(decision(branch))); + if (!refused) { + expect(issues).toEqual([]); + return; + } + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('custom'); + expect(issues[0].path).toEqual(AT_BRANCH_0); + expect(issues[0].message).toBe(predicateSlotRefusal(refusedWith)!.message); + expect(issues[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + + it('names WHICH branch: an absent second branch is anchored at index 1, the valid first one is not', () => { + const issues = issuesOf(flowWith(decision({ label: 'a', expression: 'record.amount > 10' }, { label: 'b' }))); + expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'config', 'conditions', 1, 'expression']]]); + }); + + it('reaches a `decision` inside an ADR-0031 region body, anchored where the author wrote it', () => { + const issues = issuesOf(flowWith(loopAround(decision({ label: 'y' })))); + expect(issues.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'conditions', 0, 'expression']], + ]); + expect(issues[0].message).toBe(predicateSlotRefusal(undefined)!.message); + }); + + describe('CONTROLS — what this rule must NOT reach', () => { + it('a decision that declares no branch still parses — it routes by its out-edges', () => { + expect(FlowSchema.safeParse(flowWith({ id: 'branch', type: 'decision', label: 'B', config: {} })).success).toBe(true); + expect(FlowSchema.safeParse(flowWith({ id: 'branch', type: 'decision', label: 'B' })).success).toBe(true); + expect(FlowSchema.safeParse(flowWith(decision())).success).toBe(true); + }); + + it('an absent `visibleWhen` on a screen field still parses — that slot is not required', () => { + const screen = { id: 'form', type: 'screen', label: 'Form', config: { fields: [{ name: 'amount', label: 'Amount', type: 'number' }] } }; + expect(FlowSchema.safeParse(flowWith(screen)).success).toBe(true); + }); + }); +}); diff --git a/packages/spec/src/automation/flow-node-expression-paths.test.ts b/packages/spec/src/automation/flow-node-expression-paths.test.ts index 82199f2b21e..7cb9cc4a028 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.test.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.test.ts @@ -325,10 +325,59 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu // Same rule on the other predicate slot — one class, not one node type. expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: true }] }) .map((f) => [f.path, f.value])).toEqual([['fields[0].visibleWhen', true]]); - // `null` / absent stay "not authored" — a refusal needs something authored. + // `null` / absent stay "not authored" on a slot that is NOT required — a + // refusal needs something authored. (A `required` slot is the exception, + // #19961: see the block below.) expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: null }] })).toEqual([]); }); + /** + * [#19961] A `required` predicate slot — `decision`'s + * `conditions[].expression`, which `DecisionConditionSchema` declares + * `z.string()` — emits an ABSENT or `null` value on a branch that exists, for + * every door to refuse through `predicateSlotRefusal`. It used to be skipped + * as "not authored", and the executor then evaluated the branch as a + * condition with no `source` and failed the run there. + */ + describe('a required predicate slot emits its absent value (#19961)', () => { + it('the required set is exactly the decision branch predicate — the absent arm is worded for it', () => { + // `predicateSlotRefusal`'s absent arm names a decision branch and its + // prescription; a second `required` entry must re-word it, so its + // arrival fails here rather than shipping a sentence about the wrong slot. + expect(FLOW_NODE_EXPRESSION_PATHS.filter((e) => e.required).map((e) => `${e.nodeType}.${e.path} (${e.role})`)) + .toEqual(['decision.conditions[].expression (predicate)']); + }); + + it('emits `undefined` for a branch with no `expression` key, and `null` for `expression: null`', () => { + const found = resolveFlowNodeExpressions('decision', { + conditions: [ + { label: 'first', expression: 'record.amount > 10' }, + { label: 'absent' }, + { label: 'null', expression: null }, + { label: 'aliased', condition: 'record.amount > 5' }, + ], + }); + expect(found.map((f) => [f.path, f.value, f.entry.role])).toEqual([ + ['conditions[0].expression', 'record.amount > 10', 'predicate'], + ['conditions[1].expression', undefined, 'predicate'], + ['conditions[2].expression', null, 'predicate'], + // The edge's spelling on a branch is not `expression`: the branch has none. + ['conditions[3].expression', undefined, 'predicate'], + ]); + }); + + it('judges only a branch that exists — no `conditions`, or an empty list, declares no branch', () => { + expect(resolveFlowNodeExpressions('decision', {})).toEqual([]); + expect(resolveFlowNodeExpressions('decision', { conditions: [] })).toEqual([]); + expect(resolveFlowNodeExpressions('decision', { conditions: 'nope' })).toEqual([]); + }); + + it('a slot that is NOT required keeps skipping its absent value — an absent `visibleWhen` shows the field', () => { + expect(resolveFlowNodeExpressions('screen', { fields: [{ name: 'amount' }] })).toEqual([]); + expect(resolveFlowNodeExpressions('screen', { fields: [{ name: 'amount', visibleWhen: null }] })).toEqual([]); + }); + }); + describe('predicateSlotRefusal (#15572)', () => { it('says nothing about a non-blank string — what it SAYS is validateExpression\'s business', () => { expect(predicateSlotRefusal('record.rating >= 4')).toBeUndefined(); @@ -366,6 +415,36 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu expect(predicateSlotRefusal({ source: 'x' })?.source).toBe('x'); expect(predicateSlotRefusal(42)?.source).toBe(''); }); + + /** + * [#19961] No value at all — the resolver hands one over only for a + * `required` slot, and this is the ONE refusal every door answers it with. + * The prescription is the blank's minus the run it kept (an absent branch + * predicate never evaluated — the run failed at the branch), so the + * load-bearing clauses are pinned by name. + */ + it('REFUSES no value at all — absent and `null` — under the same sentence, with the branch prescription (#19961)', () => { + const absent = predicateSlotRefusal(undefined); + const nulled = predicateSlotRefusal(null); + for (const [refusal, found] of [[absent, 'Found nothing — the key is absent'], [nulled, 'Found `null`']] as const) { + expect(refusal?.message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(refusal?.message).toContain(found); + // The prescription: write the rule; `condition` belongs in `expression`; + // `'false'` keeps the branch and never takes it; not by dropping the + // only branch. + expect(refusal?.message).toContain('Write the predicate the branch was meant to test'); + expect(refusal?.message).toContain('`condition` is the edge\'s spelling — belongs in `expression`'); + expect(refusal?.message).toContain('There is no run to keep'); + expect(refusal?.message).toContain('write `expression: \'false\'`'); + expect(refusal?.message).toContain('Not by dropping a decision\'s only branch'); + // Nothing was authored, so nothing is attributed. + expect(refusal?.source).toBe(''); + } + // Its own detail, never the envelope's or the blank's. + expect(absent?.message).not.toContain('Found a undefined'); + expect(absent?.message).not.toContain('envelope is the `value`-role spelling'); + expect(absent?.message).not.toContain('the value the blank evaluated to'); + }); }); /** * [#15662] The STRUCTURAL condition arm — `config.condition` on any node and diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 0ed1359c87e..44394d220f6 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -151,6 +151,27 @@ export interface FlowNodeExpressionPath { readonly role: FlowNodeExpressionRole; /** Author-facing label for diagnostics, e.g. `screen field visibleWhen`. */ readonly label: string; + /** + * The declaring channel REQUIRES this slot on every element the path reaches + * (#19961) — its schema lists the key in `required`, so leaving it out, or + * writing `null`, is not "not authored" but a declared slot left empty. + * + * Only meaningful for a `predicate` slot, and it changes one thing: an absent + * or `null` value on an element that exists is EMITTED by + * {@link resolveFlowNodeExpressions} (for the consumer to refuse through + * {@link predicateSlotRefusal}) instead of skipped. The element itself must + * exist — a `decision` with no `conditions` declares no branch, and nothing + * here says it must. + * + * Reconciled against the channel's own `required` list by the ratchet that + * reconciles the markers (`config-expression-ledger.test.ts` in + * `service-automation`), in both directions over the `predicate` role, so + * this flag cannot claim a requirement the contract does not make, nor miss + * one it does. Never set on another role: the channels require + * `loop.collection` / `map.collection` too, but no door refuses their + * absence — their executors parse their own config. + */ + readonly required?: true; } /** @@ -202,10 +223,18 @@ export const FLOW_NODE_EXPRESSION_PATHS: readonly FlowNodeExpressionPath[] = [ // Declared through the schemaless channel — `decision` publishes no // descriptor `configSchema`, so the marker lives on // `DecisionConditionSchema.expression`'s `.meta()` (#4439). + // + // `required` (#19961): `DecisionConditionSchema` declares `expression` + // `z.string()`, not optional, and the executor evaluates every branch it + // reaches — a branch with no `expression` hands `evaluateCondition` an + // envelope with no `source`, which it refuses, failing the run AT that + // branch. Nothing parses a node's open `config` against that schema, so + // this flag is how the requirement reaches the three doors. nodeType: 'decision', path: 'conditions[].expression', role: 'predicate', label: 'decision branch expression', + required: true, }, { nodeType: 'loop', @@ -318,7 +347,13 @@ export function isExpressionEnvelopeShaped(value: unknown): value is { dialect: * * - `predicate`: the slot IS the expression, so every string is emitted — * including one that is blank after trimming (#17493). Absent and `null` - * are skipped: "not authored" is not a malformed expression. A **non-string** + * are skipped: "not authored" is not a malformed expression — UNLESS the + * entry is {@link FlowNodeExpressionPath.required} (#19961), where an + * element that exists without its slot is a declared rule left out, and is + * emitted (as `undefined` or `null`, whichever was there) for the consumer + * to refuse through {@link predicateSlotRefusal}, the same way as a blank. + * Only the element's OWN slot is judged: an element that is not an object + * carries no slot, and the walk does not reach it. A **non-string** * is emitted too (#15572), for the consumer to refuse through * {@link predicateSlotRefusal}: it used to be skipped as "a type violation * for the schema pass to report", and for a schemaless node type there is no @@ -361,9 +396,11 @@ export function resolveFlowNodeExpressions( // consumer to refuse (see `predicateSlotRefusal`); a `flow-template` // slot keeps skipping it. if (entry.role === 'predicate' || NON_BLANK_STRING(value)) out.push({ entry, path, value }); - } else if (entry.role === 'predicate' && value != null) { + } else if (entry.role === 'predicate' && (value != null || entry.required)) { // #15572 — a non-string in a predicate slot. Emitted so a consumer can // REFUSE it (see `predicateSlotRefusal`), never so it can be parsed. + // #19961 — on a REQUIRED slot, absent and `null` are emitted too: the + // walk hands over `undefined` for a key an existing element lacks. out.push({ entry, path, value }); } }); @@ -453,11 +490,47 @@ export const PREDICATE_SLOT_STRING_REFUSAL = * register and `objectstack validate` locates it, instead of the flow running * on a predicate no validator ever read. * + * ## No value at all — refused since #19961, where the slot is required + * + * `undefined` and `null` get their own detail sentence, under the same lead. + * Whether an absent value is a finding at all is the RESOLVER's question, not + * this function's: {@link resolveFlowNodeExpressions} emits one only for a + * {@link FlowNodeExpressionPath.required} slot — today a `decision` branch's + * `expression` — and skips it everywhere else (an absent `visibleWhen` shows + * the field). So every door refuses the absent branch predicate through this + * one call, with one prescription, exactly as it refuses the blank one. + * + * The prescription is the blank's, minus the half that does not carry over. + * A blank predicate evaluated to `false`, so `expression: 'false'` KEPT its run; + * an absent one did not evaluate at all — the executor handed + * `evaluateCondition` an envelope with no `source`, which it refuses — so a + * run that reached the branch failed there, and there is no run to keep. What + * does carry over is the warning: dropping a decision's ONLY branch turns the + * node into a plain gateway, releasing the out-edge that branch labelled. + * + * ⚠️ The sentence is worded for the one required slot there is. A second + * `required` entry must re-word it — `flow-node-expression-paths.test.ts` pins + * the required set to that one entry so the second cannot arrive silently. + * * @returns the refusal and the source to attribute it to, or `undefined` when * the value is a non-blank string and therefore this function's business is * done. */ export function predicateSlotRefusal(value: unknown): { message: string; source: string } | undefined { + if (value === undefined || value === null) { + return { + message: + `${PREDICATE_SLOT_STRING_REFUSAL} Found ${value === null ? '`null`' : 'nothing — the key is absent'} ` + + 'where the slot is required: a decision branch is `{ label, expression }` and its `expression` is not ' + + 'optional, so a branch without one states no rule. Write the predicate the branch was meant to test ' + + '(e.g. `record.rating >= 4`); a predicate written under another key — `condition` is the edge\'s ' + + 'spelling — belongs in `expression`. There is no run to keep: the executor evaluates every branch it ' + + 'reaches, and a branch with no `expression` failed the run there. To keep the branch and its label but ' + + 'never take it, write `expression: \'false\'`. Not by dropping a decision\'s only branch: the node then ' + + 'routes by its out-edges alone, and the out-edge that branch labelled is no longer held back.', + source: '', + }; + } if (typeof value === 'string') { if (NON_BLANK_STRING(value)) return undefined; return { diff --git a/packages/spec/src/automation/flow-predicate-slot-blank.test.ts b/packages/spec/src/automation/flow-predicate-slot-blank.test.ts index b6ddcf7f4ee..63c9ce13ec0 100644 --- a/packages/spec/src/automation/flow-predicate-slot-blank.test.ts +++ b/packages/spec/src/automation/flow-predicate-slot-blank.test.ts @@ -107,6 +107,9 @@ describe('FlowSchema.parse refuses a blank string in a ledger predicate slot (#1 expect(FlowSchema.safeParse(flowWith(loopAround(decision('item.done')))).success).toBe(true); }); + // A screen field's `visibleWhen` is optional, and a decision with no + // `conditions` declares no branch. An ABSENT decision BRANCH predicate is + // another matter since #19961 (`flow-decision-branch-expression-absent.test.ts`). it('an absent predicate is still not a malformed one', () => { expect(FlowSchema.safeParse(flowWith(screen(undefined))).success).toBe(true); expect(FlowSchema.safeParse(flowWith({ id: 'branch', type: 'decision', label: 'B', config: {} })).success).toBe(true); @@ -114,8 +117,11 @@ describe('FlowSchema.parse refuses a blank string in a ledger predicate slot (#1 it('a NON-string in a predicate slot is not this door\'s to refuse — #15572 refuses it at the other two', () => { // Scoped to what was ruled: the flow parse's accept set moves for blank - // STRINGS only. The envelope is refused at `registerFlow` and - // `objectstack validate` by the same `predicateSlotRefusal`, unchanged. + // STRINGS only — and, since #19961, for the absent / `null` value of a + // `required` slot (`flow-decision-branch-expression-absent.test.ts`), + // where nothing was authored at all. The envelope is refused at + // `registerFlow` and `objectstack validate` by the same + // `predicateSlotRefusal`, unchanged. expect(predicateIssues(flowWith(decision({ dialect: 'cel', source: 'x > 1' })))).toEqual([]); expect(predicateIssues(flowWith(screen({ dialect: 'cel', source: ' ' })))).toEqual([]); }); diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 7785358b87b..71b1ee94ccc 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -1305,18 +1305,30 @@ export const FlowSchema = lazySchema(() => strictObject( // `decision` branch carrying it was never taken, and the two sides agreeing // was ruled no defence. // + // #19961 carries the same refusal to the ABSENT value of a REQUIRED slot — + // a `decision` branch with no `expression` key, or `expression: null`. The + // resolver emits it only where the ledger entry is `required` (an absent + // `visibleWhen` is still "not authored"), and `predicateSlotRefusal` answers + // it under the same lead sentence, with the prescription the blank carries + // minus the run it kept: an absent branch predicate never evaluated, the + // executor refused it at run time, so there was no run to keep. Triage's + // direction on that card named all three doors, this one included. + // // Scoped on purpose, three ways: - // - STRINGS only. A non-string there (the `{ dialect, source }` envelope - // above all) is refused at the other two doors by the same function - // (#15572) and was never ruled at this one; refusing it here would narrow - // the flow parse's accept set past the ruling. + // - STRINGS, plus the absent / `null` value of a `required` slot. Any other + // non-string there (the `{ dialect, source }` envelope above all) is + // refused at the other two doors by the same function (#15572) and was + // never ruled at this one; refusing it here would narrow the flow parse's + // accept set past the ruling. // - The ledger's `predicate` role only. A `flow-template` slot's blank is // untouched (no validator implements that dialect), and so is the // structural `config.condition`, which the ledger does not list — its // blank is refused at `registerFlow` and `objectstack validate` (#17322, // #17495), and a node's open `config` still carries no parse door for it. // - A VALUE rule, never a key-set closure: the node `config` stays the open - // record the header of this module describes. + // record the header of this module describes. The absent value of a + // `required` slot is the one "missing key" it reads, and only on an + // element that exists — no other key is required, none is refused. // // Walked with `collectFlowGraphs`, like the two refusals above, so a // `decision` inside an ADR-0031 region body is refused here too, anchored at @@ -1326,7 +1338,8 @@ export const FlowSchema = lazySchema(() => strictObject( const type: unknown = (node as { type?: unknown } | null)?.type; if (typeof type !== 'string') return; for (const found of resolveFlowNodeExpressions(type, (node as { config?: unknown }).config)) { - if (found.entry.role !== 'predicate' || typeof found.value !== 'string') continue; + if (found.entry.role !== 'predicate') continue; + if (typeof found.value !== 'string' && !(found.entry.required && found.value == null)) continue; const refusal = predicateSlotRefusal(found.value); if (!refusal) continue; ctx.addIssue({ diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-decision-branch-expression-absent-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-decision-branch-expression-absent-refused.ts new file mode 100644 index 00000000000..d2e7ee68f64 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-decision-branch-expression-absent-refused.ts @@ -0,0 +1,69 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// The absent half of the decision-branch predicate rule. A SEPARATE entry from +// `flow-predicate-slot-blank-string-refused` on purpose: that one keeps the +// run a blank predicate made (it evaluated `false`, so `'false'` runs the same +// route), while an absent predicate made no run to keep — the executor refused +// it at the branch — so the two carry different prescriptions, and only the +// decision branch is in this one (an absent screen `visibleWhen` stays legal). +// +// No backticks in `surface` — build-upgrade-guide.ts renders it inside a code +// span already, and a nested backtick would close it. +export const entry: SemanticMigration = { + id: 'flow-decision-branch-expression-absent-refused', + surface: + 'a decision node branch — an element of config.conditions[] — written without its expression ' + + 'key, or with expression: null, at any depth including an ADR-0031 region body. That includes ' + + 'a branch whose predicate sits under another key (condition is the edge spelling). Reachable ' + + 'wherever a flow is authored or stored: defineStack({ flows }) sources, defineFlow(), an ' + + 'exported stack passed to objectstack validate, a flow saved from the Studio flow designer ' + + 'with a branch row whose expression cell is empty, and a flow row already sitting in ' + + 'sys_metadata', + replacement: + 'the predicate the branch was meant to test, as non-blank bare CEL text under `expression` ' + + '(`{ label: \'high\', expression: \'record.amount > 10000\' }`); a predicate written under ' + + '`condition` moves to `expression`. To keep the branch and its label but never take it, ' + + 'write `expression: \'false\'` — that is a CHANGE of behaviour, not a preserved one: a run ' + + 'that reached the branch used to fail there (`condition evaluation error`), and now routes on ' + + 'to the next branch or the declared fallback. ⚠️ Not by dropping a decision\'s only branch: ' + + 'with no `conditions` the node routes by its out-edges alone, so the out-edge that branch ' + + 'labelled is no longer held back', + reason: + 'Card #19961. `DecisionConditionSchema` declares a branch `{ label, expression }` with ' + + '`expression` a required `z.string()`, but nothing parses a decision node\'s open config ' + + 'against it, and the expression-ledger resolver skipped an absent value as "not authored" — ' + + 'so a branch with no predicate passed `FlowSchema.parse`, `AutomationEngine.registerFlow` and ' + + '`objectstack validate`, and the decision executor then handed `evaluateCondition` an envelope ' + + 'with no `source`, which it refuses: the build accepted what the run refused. The ledger now ' + + 'marks the slot `required` (reconciled against that schema\'s own `required` list), the ' + + 'resolver emits the absent value there, and all three doors refuse it through ' + + '`predicateSlotRefusal`, leading with `PREDICATE_SLOT_STRING_REFUSAL` — the walk, function ' + + 'and sentence that already refuse the blank string. ' + + '⚠️ No D2 conversion: the platform cannot know the rule the author left out, and `\'false\'` ' + + 'would change what the flow does rather than keep it. ' + + '⚠️ Where such a branch already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not ' + + 'armed, while the flows beside it register; a `defineStack({ flows })` source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + + 'ADR-0087, ADR-0032.', + acceptanceCriteria: + 'Grep every flow node in `defineStack({ flows })` sources, exported stacks and every flow ' + + 'row in `sys_metadata` — including nodes inside a `loop` / `parallel` / `try_catch` region ' + + 'body — for a `decision` node whose `config.conditions[i]` has no `expression` key, or ' + + '`expression: null`. Each refusal names the node and the branch: `FlowSchema.parse` anchors ' + + 'a `custom` issue at `nodes.N.config.conditions.I.expression` (or the region path ' + + '`nodes.N.config.body.nodes.M.config…`), and `objectstack validate` prints the same path; ' + + '`validateStackExpressions` phrases it as ' + + '`node \'check\' (decision) decision branch expression at config.conditions[0].expression`. ' + + 'For each hit write the predicate the branch was meant to test, or `expression: \'false\'` ' + + 'where the branch should keep its label and never be taken. Two proofs. (1) For a stack ' + + 'authored in config files, `objectstack validate` is clean. (2) Boot the stack and confirm ' + + 'each flow REGISTERS: no `failed to register flow` warn for it (the three boot paths spell ' + + 'it `[Automation] failed to register flow`, `[Automation] flow re-sync: failed to register ' + + 'flow` and `[Automation] cold-boot flow bind: failed to register flow`) — that warn line is ' + + 'the locator for a row that exists only in `sys_metadata`. A branch carrying a non-blank ' + + 'predicate parses and registers byte-identically to before, a decision with no `conditions` ' + + 'still routes by its out-edges, and an absent screen field `visibleWhen` is still legal.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 8ded6e455fa..fded17ed07f 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -9854,6 +9854,71 @@ const step18: MigrationStep = { + 'answering the `FILTER_TEXT_CASES` stored-value row (objectstack#14079), so a ' + 'driver-level test is not evidence about this migration in either direction.', }, + // The absent half of the decision-branch predicate rule. A SEPARATE entry from + // `flow-predicate-slot-blank-string-refused` on purpose: that one keeps the + // run a blank predicate made (it evaluated `false`, so `'false'` runs the same + // route), while an absent predicate made no run to keep — the executor refused + // it at the branch — so the two carry different prescriptions, and only the + // decision branch is in this one (an absent screen `visibleWhen` stays legal). + // + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code + // span already, and a nested backtick would close it. + { + id: 'flow-decision-branch-expression-absent-refused', + surface: + 'a decision node branch — an element of config.conditions[] — written without its expression ' + + 'key, or with expression: null, at any depth including an ADR-0031 region body. That includes ' + + 'a branch whose predicate sits under another key (condition is the edge spelling). Reachable ' + + 'wherever a flow is authored or stored: defineStack({ flows }) sources, defineFlow(), an ' + + 'exported stack passed to objectstack validate, a flow saved from the Studio flow designer ' + + 'with a branch row whose expression cell is empty, and a flow row already sitting in ' + + 'sys_metadata', + replacement: + 'the predicate the branch was meant to test, as non-blank bare CEL text under `expression` ' + + '(`{ label: \'high\', expression: \'record.amount > 10000\' }`); a predicate written under ' + + '`condition` moves to `expression`. To keep the branch and its label but never take it, ' + + 'write `expression: \'false\'` — that is a CHANGE of behaviour, not a preserved one: a run ' + + 'that reached the branch used to fail there (`condition evaluation error`), and now routes on ' + + 'to the next branch or the declared fallback. ⚠️ Not by dropping a decision\'s only branch: ' + + 'with no `conditions` the node routes by its out-edges alone, so the out-edge that branch ' + + 'labelled is no longer held back', + reason: + 'Card #19961. `DecisionConditionSchema` declares a branch `{ label, expression }` with ' + + '`expression` a required `z.string()`, but nothing parses a decision node\'s open config ' + + 'against it, and the expression-ledger resolver skipped an absent value as "not authored" — ' + + 'so a branch with no predicate passed `FlowSchema.parse`, `AutomationEngine.registerFlow` and ' + + '`objectstack validate`, and the decision executor then handed `evaluateCondition` an envelope ' + + 'with no `source`, which it refuses: the build accepted what the run refused. The ledger now ' + + 'marks the slot `required` (reconciled against that schema\'s own `required` list), the ' + + 'resolver emits the absent value there, and all three doors refuse it through ' + + '`predicateSlotRefusal`, leading with `PREDICATE_SLOT_STRING_REFUSAL` — the walk, function ' + + 'and sentence that already refuse the blank string. ' + + '⚠️ No D2 conversion: the platform cannot know the rule the author left out, and `\'false\'` ' + + 'would change what the flow does rather than keep it. ' + + '⚠️ Where such a branch already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not ' + + 'armed, while the flows beside it register; a `defineStack({ flows })` source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + + 'ADR-0087, ADR-0032.', + acceptanceCriteria: + 'Grep every flow node in `defineStack({ flows })` sources, exported stacks and every flow ' + + 'row in `sys_metadata` — including nodes inside a `loop` / `parallel` / `try_catch` region ' + + 'body — for a `decision` node whose `config.conditions[i]` has no `expression` key, or ' + + '`expression: null`. Each refusal names the node and the branch: `FlowSchema.parse` anchors ' + + 'a `custom` issue at `nodes.N.config.conditions.I.expression` (or the region path ' + + '`nodes.N.config.body.nodes.M.config…`), and `objectstack validate` prints the same path; ' + + '`validateStackExpressions` phrases it as ' + + '`node \'check\' (decision) decision branch expression at config.conditions[0].expression`. ' + + 'For each hit write the predicate the branch was meant to test, or `expression: \'false\'` ' + + 'where the branch should keep its label and never be taken. Two proofs. (1) For a stack ' + + 'authored in config files, `objectstack validate` is clean. (2) Boot the stack and confirm ' + + 'each flow REGISTERS: no `failed to register flow` warn for it (the three boot paths spell ' + + 'it `[Automation] failed to register flow`, `[Automation] flow re-sync: failed to register ' + + 'flow` and `[Automation] cold-boot flow bind: failed to register flow`) — that warn line is ' + + 'the locator for a row that exists only in `sys_metadata`. A branch carrying a non-blank ' + + 'predicate parses and registers byte-identically to before, a decision with no `conditions` ' + + 'still routes by its out-edges, and an absent screen field `visibleWhen` is still legal.', + }, // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code // span already, and a nested backtick would close it. {