Skip to content

Commit 46ed703

Browse files
committed
wip(#15662): structural condition shape refusal — spec helper + both validator arms
1 parent ef60224 commit 46ed703

3 files changed

Lines changed: 176 additions & 8 deletions

File tree

‎packages/lint/src/validate-expressions.ts‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,12 @@
7979
*/
8080

8181
import { validateExpression, collectCelRootIdentifiers, parseCelToAst, SCOPE_ROOTS } from '@objectstack/formula';
82-
import { collectFlowGraphs, predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
82+
import {
83+
collectFlowGraphs,
84+
predicateSlotRefusal,
85+
resolveFlowNodeExpressions,
86+
structuralConditionRefusal,
87+
} from '@objectstack/spec/automation';
8388
// [#15137] The `value`-role half. Same two published primitives the engine
8489
// composes at `registerFlow` (`AutomationEngine.valueEnvelopeRefusals`), in the
8590
// same order: the SHAPE rule lives in the spec's `AssignmentValueSchema` (it
@@ -1156,12 +1161,44 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
11561161
}
11571162
};
11581163

1164+
/**
1165+
* [#15662] A STRUCTURAL condition (`config.condition` on a node,
1166+
* `edge.condition`) refused on SHAPE before anything reads a source out of
1167+
* it. The refusal is the spec's, shared with the engine's `registerFlow`
1168+
* pass: `error`, because that pass throws, and a shape build refuses must
1169+
* not pass author time.
1170+
*
1171+
* ⚠️ Deliberately NOT `predicateSlotRefusal`, which the declared-slot arm
1172+
* above uses. A ledger `predicate` slot is declared `z.string()`; neither
1173+
* structural slot is — `FlowEdgeSchema.condition` is
1174+
* `ExpressionInputSchema`, so an envelope is the shape the parse itself
1175+
* produces, and a node's `config` is an open `z.record` that passes one
1176+
* through verbatim. Both are admitted here; a value that is neither text
1177+
* nor an expression is not, because the evaluator reads it as an EMPTY
1178+
* condition and answers a silent `false`.
1179+
*
1180+
* @returns whether the slot was refused, so the caller can skip the
1181+
* value-reading passes that would otherwise re-report it as an empty one.
1182+
*/
1183+
const checkStructuralCondition = (where: string, raw: unknown): { refused: boolean } => {
1184+
if (raw == null) return { refused: false };
1185+
const shapeRefusal = structuralConditionRefusal(raw);
1186+
if (shapeRefusal) {
1187+
issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' });
1188+
return { refused: true };
1189+
}
1190+
return { refused: false };
1191+
};
1192+
11591193
for (const graph of graphs) {
11601194
const at = graph.scope ? `flow '${flowName}' · ${graph.scope}` : `flow '${flowName}'`;
11611195
for (const node of graph.nodes as unknown as AnyRec[]) {
11621196
const cfg = (node.config ?? {}) as AnyRec;
1163-
check(`${at} · node '${node.id}' (${node.type}) condition`, cfg.condition, objectName);
1164-
warnShadowedFieldReads(`${at} · node '${node.id}' (${node.type}) condition`, cfg.condition);
1197+
const nodeCondWhere = `${at} · node '${node.id}' (${node.type}) condition`;
1198+
if (!checkStructuralCondition(nodeCondWhere, cfg.condition).refused) {
1199+
check(nodeCondWhere, cfg.condition, objectName);
1200+
warnShadowedFieldReads(nodeCondWhere, cfg.condition);
1201+
}
11651202

11661203
// Descriptor-declared expression slots (#4027). Before this, the traversal
11671204
// hardcoded `condition` and assumed every other node string was a `{var}`
@@ -1276,8 +1313,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
12761313
}
12771314
}
12781315
for (const edge of graph.edges as unknown as AnyRec[]) {
1279-
check(`${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition, objectName);
1280-
warnShadowedFieldReads(`${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition);
1316+
const edgeCondWhere = `${at} · edge '${edge.id}' (${edge.source}→${edge.target}) condition`;
1317+
if (!checkStructuralCondition(edgeCondWhere, edge.condition).refused) {
1318+
check(edgeCondWhere, edge.condition, objectName);
1319+
warnShadowedFieldReads(edgeCondWhere, edge.condition);
1320+
}
12811321
}
12821322
// No `checkNullGuards` on node/edge conditions — and NOT for the reason
12831323
// #4811 first recorded (#4811 re-measured it). The stated blocker was the

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

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlo
2525
// `validate-flow-trigger-readiness`, so the runtime cannot drift from what
2626
// authoring accepted. See `resolveTriggerBinding`.
2727
import { resolveFlowTriggerKind } from '@objectstack/spec/automation';
28-
import { predicateSlotRefusal, resolveFlowNodeExpressions } from '@objectstack/spec/automation';
28+
import { predicateSlotRefusal, resolveFlowNodeExpressions, structuralConditionRefusal } from '@objectstack/spec/automation';
2929
// [#15137] The `value`-role half of the ledger. Both halves of "is this envelope
3030
// well-formed?" are IMPORTED, never re-spelled here: the shape rule is
3131
// `AssignmentValueSchema` (spec, #14149 — it refuses a non-`cel` dialect and the
@@ -7108,6 +7108,40 @@ export class AutomationEngine implements IAutomationService {
71087108
}
71097109
};
71107110

7111+
/**
7112+
* [#15662] The STRUCTURAL condition surfaces — `config.condition` on any
7113+
* node and `edge.condition` — refused on SHAPE before anything tries to
7114+
* read a source out of them.
7115+
*
7116+
* ⚠️ Not `predicateSlotRefusal`, the ledger arm's rule, and the
7117+
* difference is measured rather than assumed: `FlowEdgeSchema.condition`
7118+
* is `ExpressionInputSchema`, whose string arm transforms into
7119+
* `{ dialect: 'cel', source }`, so after `FlowSchema.parse` EVERY
7120+
* authored edge condition is an envelope — the ledger rule here would
7121+
* refuse every conditional edge in every flow. An envelope written at a
7122+
* node's `config.condition` is likewise passed through verbatim by the
7123+
* open `z.record` and evaluated correctly (#4336). Both are legitimate;
7124+
* `structuralConditionRefusal` admits them.
7125+
*
7126+
* What it refuses is the value that is neither text nor an expression.
7127+
* `evaluateCondition` reads `expression?.source ?? ''` and the
7128+
* empty-source arm answers `false` — "an unauthored branch must not
7129+
* open", applied to a value that was authored — so `42` / `true` /
7130+
* `['a']` registered clean and ran silently, on the same key the start
7131+
* node's trigger gate is read from. Same severity as a malformed
7132+
* predicate (this throws): the reject set of registration and the reject
7133+
* set of evaluation must be one set.
7134+
*/
7135+
const checkStructuralCondition = (where: string, raw: unknown): void => {
7136+
if (raw == null) return;
7137+
const shapeRefusal = structuralConditionRefusal(raw);
7138+
if (shapeRefusal) {
7139+
failures.push(` • ${where}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``);
7140+
return;
7141+
}
7142+
check(where, raw);
7143+
};
7144+
71117145
// #4347 — every graph in the flow, not just the top-level arrays. An
71127146
// ADR-0031 container keeps a whole sub-graph in its `config`, so
71137147
// iterating `flow.nodes`/`flow.edges` checked PART of the flow while
@@ -7120,7 +7154,7 @@ export class AutomationEngine implements IAutomationService {
71207154
for (const node of graph.nodes) {
71217155
const cfg = (node.config ?? {}) as Record<string, unknown>;
71227156
// start-node trigger gate + decision/branch predicates live in config.condition
7123-
check(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition);
7157+
checkStructuralCondition(`${at}node '${node.id}' (${node.type}) condition`, cfg.condition);
71247158

71257159
// Descriptor-declared expression slots (#4027). The ledger names them
71267160
// per node type and carries the dialect each one takes, so a declared
@@ -7177,7 +7211,7 @@ export class AutomationEngine implements IAutomationService {
71777211
}
71787212
}
71797213
for (const edge of graph.edges) {
7180-
check(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown);
7214+
checkStructuralCondition(`${at}edge '${edge.id}' (${edge.source}→${edge.target}) condition`, edge.condition as unknown);
71817215
}
71827216
}
71837217

‎packages/spec/src/automation/flow-node-expression-paths.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,100 @@ export function predicateSlotRefusal(value: unknown): { message: string; source:
386386
};
387387
}
388388

389+
/**
390+
* The one sentence a refused **structural** condition leads with (#15662) —
391+
* `config.condition` on any node and `edge.condition`, the two predicate
392+
* surfaces every flow has whether or not any ledger entry names them.
393+
*
394+
* ⚠️ Deliberately NOT {@link PREDICATE_SLOT_STRING_REFUSAL}. That one says
395+
* "bare text, an envelope is not authorable" because a ledger `predicate` slot
396+
* is *declared* `z.string()`. Neither structural slot is:
397+
*
398+
* - `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm
399+
* **transforms into** `{ dialect: 'cel', source }` — so after
400+
* `FlowSchema.parse` EVERY authored edge condition is an envelope, and the
401+
* ledger arm's rule applied here would refuse every conditional edge in
402+
* every flow.
403+
* - `FlowNodeSchema.config` is an open `z.record`, so an envelope written at
404+
* `config.condition` is passed through by the parse verbatim and evaluated
405+
* correctly by `evaluateCondition` (both spellings, by #4336's ruling).
406+
*
407+
* Both shapes are therefore legitimate here and this refusal admits them. What
408+
* it refuses is the third population, which no layer ever admitted on purpose:
409+
* a value that is neither text nor an expression.
410+
*/
411+
export const STRUCTURAL_CONDITION_SHAPE_REFUSAL =
412+
'A structural condition (`config.condition` on a node, `edge.condition`) holds either BARE CEL TEXT or an '
413+
+ 'expression envelope — an object carrying a string `source`, or an `ast`. No other shape is authorable there.';
414+
415+
/**
416+
* Why a value sitting in a structural condition slot is not authorable at all —
417+
* the SINGLE notion both consumers apply, derived once (#15662).
418+
*
419+
* `undefined` — admitted — for:
420+
*
421+
* - every **string**, including a whitespace-only one. What a non-empty string
422+
* *says* stays `validateExpression('predicate', …)`'s verdict, and a
423+
* whitespace-only condition meaning `false` is consistent on both sides and
424+
* is ruled correct, not a defect.
425+
* - absent / `null`. "Not authored" is not a malformed predicate; both callers
426+
* already return early on it, and this agrees rather than disagreeing.
427+
* - an **expression envelope**: an object carrying a string `source`, or an
428+
* `ast`. That is `ExpressionSchema`'s own rule (`.refine(e => e.source !==
429+
* undefined || e.ast !== undefined)`), read here rather than re-derived, and
430+
* it is the shape `FlowEdgeSchema` produces for every parsed edge condition.
431+
* `dialect` is not required: an envelope without one is CEL, which is what
432+
* `evaluateCondition` already does with it.
433+
*
434+
* ## What it refuses, and what that was doing before
435+
*
436+
* A number, a boolean, an array, or an object that is neither — `{ source: 1 }`,
437+
* `{ dialect: 'cel' }` with no source and no ast, `{}`. `evaluateCondition`
438+
* reads the source as `expression?.source ?? ''` and the empty-source arm
439+
* returns **`false`**: the "an unauthored branch must not open" rule, applied to
440+
* a value that was very much authored. Measured: `42`, `true` and `['a']` at a
441+
* node's `config.condition` each registered clean, executed `success: true`, and
442+
* said nothing anywhere — on the same key the **start node's trigger gate** is
443+
* read from, so a flow could be silently gated shut forever. `{ source: 1 }`
444+
* did not even get that far: it reached `exprStr.trim()` and threw a bare
445+
* `TypeError` out of the validator.
446+
*
447+
* Refusing at the producer is the contract-first half: the flow does not
448+
* register and `objectstack validate` locates it, rather than the reject set of
449+
* registration and the reject set of evaluation being two different sets.
450+
*
451+
* @returns the refusal and the source to attribute it to, or `undefined` when
452+
* the value is authorable and therefore this function's business is done.
453+
*/
454+
export function structuralConditionRefusal(
455+
value: unknown,
456+
): { message: string; source: string } | undefined {
457+
if (value == null) return undefined;
458+
if (typeof value === 'string') return undefined;
459+
if (typeof value === 'object' && !Array.isArray(value)) {
460+
const rec = value as { source?: unknown; ast?: unknown };
461+
if (typeof rec.source === 'string' || rec.ast !== undefined) return undefined;
462+
}
463+
const found = Array.isArray(value)
464+
? 'an array'
465+
: typeof value === 'object'
466+
? 'an object carrying neither a string `source` nor an `ast`'
467+
: `a ${typeof value}`;
468+
// The envelope's own `source`, when it has one, so the finding still points at
469+
// the text the author wrote rather than at an empty string. A non-string
470+
// `source` (the `{ source: 1 }` case) is exactly what is being refused, so it
471+
// cannot be the attribution.
472+
const rawSource = (value as { source?: unknown }).source;
473+
return {
474+
message:
475+
`${STRUCTURAL_CONDITION_SHAPE_REFUSAL} Found ${found}. Write the condition as bare CEL text `
476+
+ '(e.g. `record.rating >= 4`), or as an expression envelope (`{ dialect: \'cel\', source: \'…\' }`). '
477+
+ 'A value that is neither is read by the evaluator as an EMPTY condition, which answers `false` '
478+
+ 'without saying anything — and on a start node that is the trigger gate.',
479+
source: typeof rawSource === 'string' ? rawSource : '',
480+
};
481+
}
482+
389483
/**
390484
* Descend `segments` through `node`, expanding a `key[]` segment over every
391485
* element of that array and a `*` segment over every own key of that object,

0 commit comments

Comments
 (0)