From d52147eeffa24c74cf296c201fc6e1ca5ddebdb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 01:32:37 +0000 Subject: [PATCH 1/2] service-automation: refuse a CEL envelope in a `z.string()` predicate slot at build time (#15572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `decision` branch predicate (or a screen field's `visibleWhen`) authored as the `{ dialect, source }` expression envelope reached a shape neither validator could see: the node's `config` is an open `z.record` no Zod schema is parsed against, `validateNodeConfigKeys` exempts the schemaless node types on purpose, and the expression ledger's `predicate` arm skipped every non-string as "a type violation for the schema pass to report" — for `decision`, a schema pass that does not exist. The evaluator accepted it regardless, so the only layer that ever read the predicate was the last one. Fixed at the producer, per the contract-first rule: - `resolveFlowNodeExpressions` emits a non-string in a `predicate` slot so a consumer can refuse it; `predicateSlotRefusal` / `PREDICATE_SLOT_STRING_REFUSAL` are the single shared notion of why. - `registerFlow` throws, located and attributed; `objectstack validate` reports the same refusal as an `error`. String behaviour is untouched: a whitespace-only string still means "not authored" on both sides, and what a non-empty string says is still `validateExpression`'s verdict. `flow-template` slots keep the old rule — no validator implements that dialect. `evaluateCondition`'s empty-source arm is deliberately NOT touched: that is the shared evaluator, and the whitespace-source envelope it turns on is #15430's surface. Part of #15572 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../decision-predicate-envelope-refused.md | 16 ++ .../lint/src/validate-expressions.test.ts | 71 ++++++- packages/lint/src/validate-expressions.ts | 22 ++- .../builtin/config-expression-ledger.test.ts | 17 +- .../src/decision-predicate-envelope.test.ts | 174 ++++++++++++++++++ .../services/service-automation/src/engine.ts | 20 +- .../flow-node-expression-paths.test.ts | 62 ++++++- .../automation/flow-node-expression-paths.ts | 88 ++++++++- 8 files changed, 449 insertions(+), 21 deletions(-) create mode 100644 .changeset/decision-predicate-envelope-refused.md create mode 100644 packages/services/service-automation/src/decision-predicate-envelope.test.ts diff --git a/.changeset/decision-predicate-envelope-refused.md b/.changeset/decision-predicate-envelope-refused.md new file mode 100644 index 0000000000..2d29f4e39b --- /dev/null +++ b/.changeset/decision-predicate-envelope-refused.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-automation": minor +"@objectstack/lint": minor +--- + +A flow predicate authored as a CEL envelope is now refused at build time, instead of running unread by either validator. + +A `predicate`-role expression slot holds **bare CEL text** — `DecisionConditionSchema.expression` is declared `z.string()`, and so is a screen field's `visibleWhen`. An author who instead wrote the `{ dialect, source }` expression *envelope* there reached a shape nothing could see: a flow node's `config` is an open `z.record(z.unknown())` that no Zod schema is parsed against, the unknown-key walk exempts the schemaless node types on purpose (`decision` publishes no descriptor `configSchema`), and the expression ledger's `predicate` arm skipped every non-string as "a type violation for the schema pass to report" — a schema pass that, for those node types, does not exist. `registerFlow` accepted the flow, `objectstack validate` reported nothing, and the evaluator was the only layer that ever read the predicate. + +- `resolveFlowNodeExpressions` now emits a non-string sitting in a `predicate` slot, and the new `predicateSlotRefusal` / `PREDICATE_SLOT_STRING_REFUSAL` say why it is refused — one notion, derived once, read by both validators so build time and author time cannot disagree about the shape. `flow-template` slots keep the old rule: no validator implements that dialect, so a finding there is one nobody could judge. +- `registerFlow` throws, naming the node, the slot and the index, and attributing the finding to the envelope's own `source`. `objectstack validate` reports the same refusal as a located `error`. + +**String predicates are untouched, deliberately.** A whitespace-only string still means "not authored" on both sides, exactly as before; what a non-empty string *says* is still judged by `validateExpression('predicate', …)`, brace trap and all. Only the shape moved. + +An app that authored an envelope in one of these slots now fails to register with a message naming the slot; the fix is to write the predicate as bare CEL text (`record.rating >= 4`). The `{ dialect, source }` envelope remains the `value`-role spelling, on the `assignment` node's `assignments` map. diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index f122a45ead..8a3544333d 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -11,7 +11,7 @@ import { FieldSchema, ObjectSchema, SelectOptionSchema } from '@objectstack/spec import { SharingRuleSchema } from '@objectstack/spec/security'; // [#15137] The published refusal sentence a `value`-slot finding must lead // with — asserted from the spec's own export, never re-spelled in a test. -import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL } from '@objectstack/spec/automation'; +import { ASSIGNMENT_VALUE_ENVELOPE_REFUSAL, PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation'; import { validateStackExpressions, @@ -2117,6 +2117,69 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(issues.filter(i => i.where.includes('conditions'))).toHaveLength(0); }); + /** + * [#15572] The same slot, authored as a CEL **envelope** rather than as the + * bare CEL text it is declared to hold (`DecisionConditionSchema.expression` + * is `z.string()`). Before this, `objectstack validate` reported nothing: + * the ledger's `predicate` arm emitted strings only, skipping the envelope + * as "a type violation for the schema pass to report" — and `decision` + * publishes no descriptor `configSchema`, so no schema pass ever ran. + * + * ⚠️ Read the RED CONTROL below before trusting any zero here. The reading + * this card was filed on used a second envelope as its control — a control + * that could itself have been the answer, which makes it no control at all + * — so this test drives a form that DOES report (the brace-trap string, on + * this very slot, through this very call) in the same run as the forms + * under test. Without it, a harness that reached the slot and a harness + * that reached nothing would read identically. + */ + describe('a predicate slot authored as an expression envelope (#15572)', () => { + const atSlot = (expression: unknown) => + validateStackExpressions(decisionFlow(expression as string)) + .filter(i => i.where.includes('conditions[0].expression')); + + it('RED CONTROL — the brace-trap string on this slot still reports', () => { + // Not an envelope, so it cannot be "the answer" to what is being + // measured; it proves only that this call reaches this slot and can + // emit. If this ever goes to zero, every zero below is void. + const control = atSlot("{lead_record.status} == 'converted'"); + expect(control).toHaveLength(1); + expect(control[0].severity).toBe('error'); + expect(control[0].message).toContain('template brace'); + }); + + it('reports a `{ dialect, source }` envelope, whatever the source says', () => { + for (const envelope of [ + { dialect: 'cel', source: ' ' }, // the silent-`false` shape + { dialect: 'cel', source: 'rows.map(r,' }, // the throwing shape + { dialect: 'cel', ast: { kind: 'const' } }, // no `source` at all + ]) { + const found = atSlot(envelope); + expect(found).toHaveLength(1); + expect(found[0].severity).toBe('error'); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(found[0].where).toContain('decision branch expression'); + } + }); + + it('attributes the finding to the envelope’s own source when it has one', () => { + expect(atSlot({ dialect: 'cel', source: 'rows.map(r,' })[0].source).toBe('rows.map(r,'); + expect(atSlot({ dialect: 'cel', ast: { kind: 'const' } })[0].source).toBe(''); + }); + + it('reports every other non-string on the same rule, naming what it found', () => { + expect(atSlot(42)[0].message).toContain('Found a number'); + expect(atSlot(['a > 1'])[0].message).toContain('Found an array'); + }); + + it('leaves string predicates alone — including the whitespace-only one', () => { + // The card states this boundary explicitly so nobody "fixes" it: a + // whitespace-only STRING is "not authored" on both sides and stays so. + expect(atSlot(' ')).toHaveLength(0); + expect(atSlot("lead_record.status == 'converted'")).toHaveLength(0); + }); + }); + it('leaves a correct single-brace loop collection alone', () => { // `loop.collection` is the single-brace `{var}` flow-interpolation dialect, // where braces are CORRECT. It is recorded in the ledger as `flow-template` @@ -2850,6 +2913,12 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t // keys; the metadata keys it judges are `AssignmentValueSchema`'s, read // by the schema and not by name here. 'shape', + // [#15572] The spec's shared refusal for a non-string in a predicate + // slot. Its keys are that helper's own `{ message, source }` — never + // metadata keys — and it is named to stay clear of the `message` / + // `source` receivers for the reason the entry above it records: a local + // called `message` here would be excused into masking a genuine read. + 'shapeRefusal', // [#14089] NOT a receiver at all — the tail of the `'./flow-variable-scope.js'` // import specifier, which this scan cannot tell from `scope.j…`. The two // entries above it in this set (`fields`, `guards`) are the same artefact diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 49411669ec..b2ffa07470 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -79,7 +79,7 @@ */ import { validateExpression, collectCelRootIdentifiers, parseCelToAst, SCOPE_ROOTS } from '@objectstack/formula'; -import { collectFlowGraphs, resolveFlowNodeExpressions } from '@objectstack/spec/automation'; +import { collectFlowGraphs, predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation'; // [#15137] The `value`-role half. Same two published primitives the engine // composes at `registerFlow` (`AutomationEngine.valueEnvelopeRefusals`), in the // same order: the SHAPE rule lives in the spec's `AssignmentValueSchema` (it @@ -1065,11 +1065,22 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { * bind the *screen's own* collected values, not the trigger record's fields, so * a field-existence pass would report every field name as unknown. */ - const checkDeclaredPredicate = (where: string, raw: unknown): void => { - if (raw == null) return; + const checkDeclaredPredicate = (where: string, raw: unknown): { refused: boolean } => { + if (raw == null) return { refused: false }; + // [#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, + // shared with the engine's `registerFlow` pass: `error`, because that pass + // throws, and a shape build refuses must not pass author time. + const shapeRefusal = predicateSlotRefusal(raw); + if (shapeRefusal) { + issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' }); + return { refused: true }; + } const res = validateExpression('predicate', raw as string | { dialect?: string; source?: string }); for (const e of res.errors) issues.push({ where, message: e.message, source: e.source, severity: 'error' }); for (const w of res.warnings) issues.push({ where, message: w.message, source: w.source, severity: 'warning' }); + return { refused: false }; }; /** @@ -1184,7 +1195,10 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { continue; } if (found.entry.role !== 'predicate') continue; - checkDeclaredPredicate(slotWhere, found.value); + // [#15572] A slot refused on SHAPE gets no second diagnostic: the + // shadowing warning is about which scope a CEL source resolves in, + // and a value that is not CEL text has no source to resolve. + if (checkDeclaredPredicate(slotWhere, found.value).refused) continue; // [#14288] The shadowing warning is about the SCOPE an expression is // evaluated in, not about which key it was authored under — so it // belongs on every `predicate` slot the ledger declares, not just the 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 cef90e3dc9..72625d2214 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 @@ -281,17 +281,26 @@ describe('resolveFlowNodeExpressions — path resolution (#4027)', () => { expect(found[0].entry.role).toBe('flow-template'); }); - it('skips absent, empty and non-string values rather than inventing findings', () => { + it('skips absent and empty values rather than inventing findings', () => { expect(resolveFlowNodeExpressions('screen', {})).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] })).toEqual([]); - // A non-string in an expression slot is a type violation for the schema pass - // to report — not something to hand to a parser. - expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: true }] })).toEqual([]); // A repeater authored as a non-array must not throw. expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]); }); + // [#15572] A NON-string in a predicate slot is no longer skipped. It was + // skipped as "a type violation for the schema pass to report", and for the + // schemaless node types — `decision` publishes no descriptor `configSchema`, + // so `validateNodeConfigKeys` exempts it and nothing parses its config + // against `DecisionConfigSchema` — that schema pass does not exist. The value + // is emitted so `registerFlow` and `objectstack validate` can refuse it. + it('emits a non-string in a predicate slot for the consumer to refuse (#15572)', () => { + expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: true }] }) + .map((f) => [f.path, f.value, f.entry.role])) + .toEqual([['fields[0].visibleWhen', true, 'predicate']]); + }); + it('resolves each decision branch predicate, with its index (#4439)', () => { const found = resolveFlowNodeExpressions('decision', { conditions: [ diff --git a/packages/services/service-automation/src/decision-predicate-envelope.test.ts b/packages/services/service-automation/src/decision-predicate-envelope.test.ts new file mode 100644 index 0000000000..b08c49c3f6 --- /dev/null +++ b/packages/services/service-automation/src/decision-predicate-envelope.test.ts @@ -0,0 +1,174 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +// The published refusal sentence, asserted from the spec's own export and +// never re-spelled here: one notion of "a predicate slot takes text", derived +// once and read by both validators. +import { PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation'; + +import { AutomationServicePlugin } from './plugin.js'; +import type { AutomationEngine } from './engine.js'; + +/** + * [#15572] A `decision` branch predicate authored as a CEL **envelope** — + * `{ dialect, source }` in a slot declared `z.string()`. + * + * Four declarations left it invisible to everyone with an opinion: + * `DecisionConditionSchema.expression` is `z.string()` but a node's `config` is + * an open `z.record` that schema is never parsed against; the unknown-key walk + * exempts the schemaless node types (`decision` publishes no descriptor + * `configSchema` on purpose); the expression ledger's `predicate` arm emitted + * strings only, skipping the envelope as "a type violation for the schema pass" + * that, for those types, does not exist; and `evaluateCondition` accepts the + * envelope. So the flow registered clean, `objectstack validate` reported + * nothing, and the only layer that ever read the predicate was the evaluator. + * + * Measured on the unfixed engine, driven end to end (`registerFlow` then + * `execute`), and worth recording because it corrects the filed symptom: through + * the DECISION door the run did not take the other branch silently — the + * executor wraps the authored value as `{ dialect: 'cel', source: }`, + * so `evaluateCondition` read a non-string `source` and the run failed with + * `exprStr.trim is not a function`, an unattributed TypeError with no source and + * no location. The silent `false` is real one door over — `evaluateCondition` + * called with the envelope directly, which is how an `edge.condition` reaches it + * — and that arm is deliberately NOT touched here: it is the shared evaluator, + * and the whitespace-source envelope it turns on belongs to #15430. + * + * The fix is at the producer, per the contract-first rule: the flow does not + * register. Both run-time shapes are then unreachable from authored metadata. + */ +describe('decision branch predicate — envelope in a `z.string()` slot (#15572)', () => { + let engine: AutomationEngine; + + beforeEach(async () => { + const kernel = new LiteKernel(); + kernel.use(new AutomationServicePlugin()); + await kernel.bootstrap(); + engine = kernel.getService('automation') as AutomationEngine; + }); + + const decisionFlow = (name: string, expression: unknown) => ({ + name, + label: name, + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'check', type: 'decision', label: 'Check', config: { conditions: [{ label: 'yes', expression }] } }, + { id: 'yes_node', type: 'assignment', label: 'Yes', config: { assignments: { took: 'yes' } } }, + { id: 'no_node', type: 'assignment', label: 'No', config: { assignments: { took: 'no' } } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'check' }, + { id: 'e1', source: 'check', target: 'yes_node', label: 'yes' }, + { id: 'e2', source: 'check', target: 'no_node', label: 'default', isDefault: true }, + { id: 'e3', source: 'yes_node', target: 'end' }, + { id: 'e4', source: 'no_node', target: 'end' }, + ], + }); + + /** + * The serious half as filed: a whitespace-only SOURCE inside the envelope. + * Nothing downstream can tell it apart from "no predicate at all", so the + * only place it can be caught is here, on the shape. + */ + it('refuses a whitespace-source envelope at registration', () => { + const register = () => engine.registerFlow('ws_flow', decisionFlow('ws_flow', { dialect: 'cel', source: ' ' })); + expect(register).toThrow(PREDICATE_SLOT_STRING_REFUSAL); + // Located: which node, which slot, which index. + expect(register).toThrow(/node 'check' \(decision\) decision branch expression at config\.conditions\[0\]\.expression/); + expect(register).toThrow(/an expression envelope/); + }); + + it('refuses an un-parseable-source envelope at registration', () => { + const register = () => engine.registerFlow('bad_flow', decisionFlow('bad_flow', { dialect: 'cel', source: 'rows.map(r,' })); + expect(register).toThrow(PREDICATE_SLOT_STRING_REFUSAL); + // The envelope's own source is attributed, so the author is pointed at + // the text they wrote rather than at an empty string. + expect(register).toThrow(/source: `rows\.map\(r,`/); + }); + + it('refuses an `ast`-only envelope, and every other non-string, on the same rule', () => { + // NOT a claim about `ExpressionSchema` accepting `source`-or-`ast` + // (#15430, a different card and a different mechanism): this refusal is + // about the SHAPE reaching a slot declared as text, whatever is in it. + expect(() => engine.registerFlow('ast_flow', decisionFlow('ast_flow', { dialect: 'cel', ast: { kind: 'const' } }))) + .toThrow(PREDICATE_SLOT_STRING_REFUSAL); + expect(() => engine.registerFlow('num_flow', decisionFlow('num_flow', 42))) + .toThrow(/Found a number/); + expect(() => engine.registerFlow('bool_flow', decisionFlow('bool_flow', true))) + .toThrow(/Found a boolean/); + }); + + /** + * The boundary the card states explicitly, so nobody "fixes" it: a + * whitespace-only STRING predicate behaves consistently on both sides — + * "not authored" at the ledger, `false` at the evaluator — and is left + * exactly as it was. Only the envelope shape moved. + */ + it('leaves string predicates alone — including the whitespace-only one', () => { + expect(() => engine.registerFlow('str_ok', decisionFlow('str_ok', 'record.rating >= 4'))).not.toThrow(); + expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', ' '))).not.toThrow(); + expect(() => engine.registerFlow('str_absent', decisionFlow('str_absent', undefined))).not.toThrow(); + }); + + /** + * The pre-existing arm still issues its own verdict: a refusal on shape + * must not swallow the #1491 brace-trap a malformed STRING earns. + */ + it('still reports a brace-in-CEL string predicate as the brace trap, not as a shape refusal', () => { + const register = () => engine.registerFlow('brace_flow', decisionFlow('brace_flow', '{record.rating} >= 4')); + expect(register).toThrow(/template braces|bare CEL/); + expect(register).not.toThrow(PREDICATE_SLOT_STRING_REFUSAL); + }); + + /** One class, not one node type — `screen.fields[].visibleWhen` is the other predicate slot. */ + it('refuses an envelope in a screen field `visibleWhen` on the same rule', () => { + const register = () => engine.registerFlow('screen_flow', { + name: 'screen_flow', + label: 'screen_flow', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'form', type: 'screen', label: 'Form', + config: { fields: [{ name: 'amount', type: 'number', visibleWhen: { dialect: 'cel', source: 'x == 1' } }] }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e0', source: 'start', target: 'form' }, + { id: 'e1', source: 'form', target: 'end' }, + ], + }); + expect(register).toThrow(PREDICATE_SLOT_STRING_REFUSAL); + expect(register).toThrow(/screen field visibleWhen at config\.fields\[0\]\.visibleWhen/); + }); + + /** + * The run-time consequence, closed at the producer: the flow never enters + * the registry, so there is nothing to execute. Asserting through + * `execute()` rather than by re-reading the registry keeps the pin on the + * behaviour that mattered — a run that decided a branch on a predicate no + * validator had read. + */ + it('leaves nothing to run: the refused flow is not registered', async () => { + expect(() => engine.registerFlow('never', decisionFlow('never', { dialect: 'cel', source: ' ' }))).toThrow(); + const result = await engine.execute('never'); + expect(result.success).toBe(false); + expect(String(result.error)).toMatch(/not found|not registered|Flow/i); + }); + + /** + * The same flow with a plain string predicate still registers AND still + * runs to completion — the refusal is scoped to the shape, and a run + * through the decision executor is what it is scoped away from. + */ + it('still registers and runs a plain string predicate', async () => { + expect(() => engine.registerFlow('runs', decisionFlow('runs', 'true'))).not.toThrow(); + const result = await engine.execute('runs'); + expect(result.success).toBe(true); + expect(result.error).toBeUndefined(); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index d765ab2e0c..f8fc575ee2 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -25,7 +25,7 @@ import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlo // `validate-flow-trigger-readiness`, so the runtime cannot drift from what // authoring accepted. See `resolveTriggerBinding`. import { resolveFlowTriggerKind } from '@objectstack/spec/automation'; -import { resolveFlowNodeExpressions } from '@objectstack/spec/automation'; +import { predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation'; // [#15137] The `value`-role half of the ledger. Both halves of "is this envelope // well-formed?" are IMPORTED, never re-spelled here: the shape rule is // `AssignmentValueSchema` (spec, #14149 — it refuses a non-`cel` dialect and the @@ -7152,6 +7152,24 @@ export class AutomationEngine implements IAutomationService { continue; } if (found.entry.role !== 'predicate') continue; + // [#15572] A predicate slot is declared bare CEL TEXT + // (`z.string()`), and until now a NON-string there was seen + // by nobody: the resolver skipped it for "the schema pass" + // to report, `validateNodeConfigKeys` exempts the schemaless + // types that own two of these slots, and `evaluateCondition` + // accepted the `{ dialect, source }` envelope regardless — + // so a `decision` branch predicate written as an envelope + // registered clean and only the evaluator ever read it. + // Same severity as a malformed predicate (this throw): the + // reject set of registration and the reject set of + // evaluation must be one set, and the refusal itself is the + // spec's, shared with `objectstack validate` so build and + // author time cannot disagree about the shape. + const shapeRefusal = predicateSlotRefusal(found.value); + if (shapeRefusal) { + failures.push(` • ${slotWhere}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``); + continue; + } // No schema hint: a screen's `visibleWhen` binds the screen's OWN // collected values, not the trigger record's fields, so the // field-existence pass would report every field name as unknown. 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 f85cb50da6..af4e05d062 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.test.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.test.ts @@ -20,6 +20,8 @@ import { FLOW_NODE_EXPRESSION_PATHS, isExpressionEnvelopeShaped, resolveFlowNodeExpressions, + predicateSlotRefusal, + PREDICATE_SLOT_STRING_REFUSAL, type FlowNodeExpressionPath, type FlowNodeExpressionRole, } from './flow-node-expression-paths.js'; @@ -178,16 +180,66 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu expect(found.every((f) => f.entry.role === 'predicate')).toBe(true); }); - it('absent, empty and non-string values in a string-role slot are still skipped', () => { + it('absent and empty values in a string-role slot are skipped', () => { expect(resolveFlowNodeExpressions('screen', {})).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]); + // A whitespace-only STRING is "not authored", on this side and at the + // evaluator alike — consistent on both sides, and deliberately left alone + // (#15572 changed the non-string rule, never the string one). expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] })).toEqual([]); - expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: true }] })).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]); - // An ENVELOPE in a predicate-role slot is a non-string — skipped here, a - // type violation for the schema pass, exactly as before: the value-role - // emission rule is scoped to `value` entries and leaks into no other role. + // A `flow-template` slot keeps the old rule: no validator implements that + // dialect, so emitting a non-string there would hand every consumer a + // finding none of them can judge. expect(resolveFlowNodeExpressions('loop', { collection: { dialect: 'cel', source: 'x' } })).toEqual([]); expect(resolveFlowNodeExpressions('decision', { condition: 'a == b' })).toEqual([]); }); + + /** + * [#15572] A NON-string in a `predicate` slot is emitted, so a consumer can + * refuse it. It used to be skipped as "a type violation for the schema pass + * to report" — and for `decision`, a schemaless node type whose config is + * never parsed against any Zod schema, there is no schema pass, so the value + * reached the evaluator with no validator having ever seen it. + */ + it('emits a non-string in a predicate slot, so a consumer can refuse it (#15572)', () => { + const envelope = { dialect: 'cel', source: ' ' }; + const decision = resolveFlowNodeExpressions('decision', { + conditions: [{ label: 'Yes', expression: envelope }], + }); + expect(decision.map((f) => [f.path, f.value, f.entry.role])) + .toEqual([['conditions[0].expression', envelope, 'predicate']]); + // 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. + expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: null }] })).toEqual([]); + }); + + describe('predicateSlotRefusal (#15572)', () => { + it('says nothing about a string — what it SAYS is validateExpression\'s business', () => { + expect(predicateSlotRefusal('record.rating >= 4')).toBeUndefined(); + // Including a string that is itself malformed: the shape is right, so + // this function is done and the CEL parse issues the verdict. + expect(predicateSlotRefusal('{record.rating} >= 4')).toBeUndefined(); + expect(predicateSlotRefusal('')).toBeUndefined(); + }); + + it('refuses an envelope and attributes it to the envelope\'s own source', () => { + const refusal = predicateSlotRefusal({ dialect: 'cel', source: 'rows.map(r,' }); + expect(refusal?.message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(refusal?.message).toContain('an expression envelope'); + expect(refusal?.source).toBe('rows.map(r,'); + }); + + it('refuses every other non-string, naming what it found', () => { + expect(predicateSlotRefusal(42)?.message).toContain('a number'); + expect(predicateSlotRefusal(true)?.message).toContain('a boolean'); + expect(predicateSlotRefusal(['a'])?.message).toContain('an array'); + expect(predicateSlotRefusal({ source: 'x' })?.message).toContain('an object'); + // No `dialect` ⇒ not envelope-shaped, so no source is claimed from it. + expect(predicateSlotRefusal({ source: 'x' })?.source).toBe('x'); + expect(predicateSlotRefusal(42)?.source).toBe(''); + }); + }); }); diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 1354d27894..6c8b487777 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -279,11 +279,20 @@ export function isExpressionEnvelopeShaped(value: unknown): value is { dialect: * * What counts as "an authored expression" depends on the role: * - * - `predicate` / `flow-template`: the slot IS the expression, so a non-empty - * string is emitted. Absent, `null` and non-string values are skipped: "not - * authored" is not a malformed expression, and a non-string in an expression - * slot is a *type* violation for the schema pass to report, not something to - * hand to a parser. + * - `predicate`: the slot IS the expression, so a non-empty string is emitted. + * Absent and `null` are skipped — "not authored" is not a malformed + * expression — and so is a whitespace-only string, which both validators and + * the evaluator already agree means the same thing. 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 + * schema pass — `decision` publishes no descriptor `configSchema`, and + * `DecisionConditionSchema` is never parsed against a node's open + * `z.record` config — so nothing reported it and the value ran anyway. + * - `flow-template`: same string rule, minus the refusal. No validator + * implements that dialect (see {@link FlowNodeExpressionRole}), so emitting a + * non-string there would hand every consumer a finding none of them can + * judge. * - `value`: the slot holds a VALUE that may be spelled as an expression, so * only envelope-shaped objects ({@link isExpressionEnvelopeShaped}) are * emitted. A string there is `{token}` interpolation — the `flow-template` @@ -302,7 +311,11 @@ export function resolveFlowNodeExpressions( walk(config as Record, entry.path.split('.'), '', (path, value) => { if (entry.role === 'value') { if (isExpressionEnvelopeShaped(value)) out.push({ entry, path, value }); - } else if (typeof value === 'string' && value.trim()) { + } else if (typeof value === 'string') { + if (value.trim()) out.push({ entry, path, value }); + } else if (entry.role === 'predicate' && value != null) { + // #15572 — a non-string in a predicate slot. Emitted so a consumer can + // REFUSE it (see `predicateSlotRefusal`), never so it can be parsed. out.push({ entry, path, value }); } }); @@ -310,6 +323,69 @@ export function resolveFlowNodeExpressions( return out; } +/** + * The one sentence a refused `predicate` slot leads with (#15572) — the same + * words however the value is wrong, so an author (or an agent reading the + * failure) learns the rule before the detail. Mirrors + * `ASSIGNMENT_VALUE_ENVELOPE_REFUSAL`, the `value` role's equivalent. + */ +export const PREDICATE_SLOT_STRING_REFUSAL = + 'A predicate slot holds BARE CEL TEXT — it is declared `z.string()` — so an expression envelope, or any other ' + + 'non-string, is not authorable there.'; + +/** + * Why a value sitting in a `predicate`-role slot is not authorable at all — + * the SINGLE notion both consumers apply, derived once (#15572). + * + * `undefined` for every string: what a string *says* is judged by + * `validateExpression('predicate', …)`, unchanged, and a whitespace-only string + * never reaches here (the resolver treats it as "not authored", the same way + * the evaluator does — deliberately, and not a defect on either side). + * + * ## Why this is a refusal and not a parse + * + * The `{ dialect, source }` envelope is the `value` role's spelling + * (`assignment.assignments.*`, the 2026-09-02 ruling on #14149). In a predicate + * slot it was a shape NOBODY could see: the slot is declared `z.string()`, but + * a node's `config` is an open `z.record(z.unknown())` that no Zod schema is + * parsed against, the unknown-key walk exempts the schemaless node types + * (`decision` publishes no descriptor `configSchema` on purpose), and this + * resolver skipped every non-string as "a type violation for the schema pass to + * report" — a schema pass that, for those types, does not exist. The evaluator + * accepted it regardless, so the one layer with an opinion was the last one. + * + * Refusing it here is the producer-side half of that: the flow does not + * register and `objectstack validate` locates it, instead of the flow running + * on a predicate no validator ever read. + * + * @returns the refusal and the source to attribute it to, or `undefined` when + * the value is a string and therefore this function's business is done. + */ +export function predicateSlotRefusal(value: unknown): { message: string; source: string } | undefined { + if (typeof value === 'string') return undefined; + const envelope = isExpressionEnvelopeShaped(value); + const found = envelope + ? 'an expression envelope (an object naming a `dialect`)' + : Array.isArray(value) + ? 'an array' + : value === null + ? '`null`' + : typeof value === 'object' + ? 'an object' + : `a ${typeof value}`; + // The envelope's own `source`, when it has one, so the finding still points + // at the text the author wrote rather than at an empty string. + const rawSource = (value as { source?: unknown } | null | undefined)?.source; + return { + message: + `${PREDICATE_SLOT_STRING_REFUSAL} Found ${found}. Write the predicate as bare CEL text ` + + '(e.g. `record.rating >= 4`); the `{ dialect, source }` envelope is the `value`-role spelling ' + + '(the `assignment` node\'s `assignments` map), and in a predicate slot it is read by the evaluator ' + + 'but by neither validator.', + source: typeof rawSource === 'string' ? rawSource : '', + }; +} + /** * Descend `segments` through `node`, expanding a `key[]` segment over every * element of that array and a `*` segment over every own key of that object, From 884267e000b9c8a2803faf899436676d8f240e07 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 01:44:08 +0000 Subject: [PATCH 2/2] spec: regenerate the api-surface and export-origins shards for the two new automation exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `predicateSlotRefusal` / `PREDICATE_SLOT_STRING_REFUSAL` are additive — 0 breaking, 2 added — and both artifacts are generated products, not hand-edited ledgers: `check:api-surface` and `check:export-origins` name the exact `gen:*` command that writes them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- packages/spec/api-surface/automation.json | 2 ++ packages/spec/export-origins/automation.json | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index df0e204a82..c430989812 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -178,6 +178,7 @@ "ORG_MEMBERSHIP_LEVELS (const)", "OS_CONSTRUCT_EXT (const)", "PARALLEL_NODE_TYPE (const)", + "PREDICATE_SLOT_STRING_REFUSAL (const)", "ParallelBranch (type)", "ParallelBranchParsed (type)", "ParallelBranchSchema (const)", @@ -259,6 +260,7 @@ "normalizeDecisionOutputs (function)", "normalizeFlowFunctionEntry (function)", "parseFlowNodeRegions (function)", + "predicateSlotRefusal (function)", "resolveFlowNodeExpressions (function)", "resolveFlowTriggerKind (function)", "validateControlFlow (function)" diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index 95da3dce08..e5eed32d4d 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -178,6 +178,7 @@ "ORG_MEMBERSHIP_LEVELS": "src/automation/approval.zod.ts#ORG_MEMBERSHIP_LEVELS (const)", "OS_CONSTRUCT_EXT": "src/automation/bpmn-mapping.ts#OS_CONSTRUCT_EXT (const)", "PARALLEL_NODE_TYPE": "src/automation/control-flow.zod.ts#PARALLEL_NODE_TYPE (const)", + "PREDICATE_SLOT_STRING_REFUSAL": "src/automation/flow-node-expression-paths.ts#PREDICATE_SLOT_STRING_REFUSAL (const)", "ParallelBranch": "src/automation/control-flow.zod.ts#ParallelBranch (type)", "ParallelBranchParsed": "src/automation/control-flow.zod.ts#ParallelBranchParsed (type)", "ParallelBranchSchema": "src/automation/control-flow.zod.ts#ParallelBranchSchema (const)", @@ -259,6 +260,7 @@ "normalizeDecisionOutputs": "src/automation/approval.zod.ts#normalizeDecisionOutputs (function)", "normalizeFlowFunctionEntry": "src/automation/flow-function.zod.ts#normalizeFlowFunctionEntry (function)", "parseFlowNodeRegions": "src/automation/control-flow.zod.ts#parseFlowNodeRegions (function)", + "predicateSlotRefusal": "src/automation/flow-node-expression-paths.ts#predicateSlotRefusal (function)", "resolveFlowNodeExpressions": "src/automation/flow-node-expression-paths.ts#resolveFlowNodeExpressions (function)", "resolveFlowTriggerKind": "src/automation/flow-trigger-kind.ts#resolveFlowTriggerKind (function)", "validateControlFlow": "src/automation/control-flow.zod.ts#validateControlFlow (function)"