diff --git a/.changeset/flow-edge-condition-evaluated-slot.md b/.changeset/flow-edge-condition-evaluated-slot.md new file mode 100644 index 0000000000..f36ffd00ed --- /dev/null +++ b/.changeset/flow-edge-condition-evaluated-slot.md @@ -0,0 +1,108 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec)!: `FlowEdgeSchema.condition` is an evaluated slot — it composes the new `EvaluatedExpressionInputSchema`, and `structuralConditionRefusal` no longer admits an `ast`-only envelope (#15807) + + + +**BREAKING** in the accept-set sense, landing in the launch window as `minor` +(the lockstep convention: `major` is refused by `check-changeset-no-major`, and +breaking-ness is carried by this banner plus the ADR-0087 disposition): the +edge condition of a flow — `FlowEdgeSchema.condition`, the branch predicate +`AutomationEngine.evaluateCondition` runs at every traversal — now refuses at +authoring an envelope the engine cannot evaluate, where it used to parse, +register, pass `objectstack validate`, and then answer a **silent `false`**: a +branch that quietly never fired. + +Two spellings of one seam, refused by ONE rule with one sentence +(`EVALUATED_EXPRESSION_SOURCE_REQUIRED`, the rule #15430 introduced for the +`assignment` value envelope): + +```yaml +edges: + - { id: e1, source: check, target: approve, condition: { dialect: cel, ast: { kind: const, value: true } } } # `ast` only — the engine never reads it + - { id: e2, source: check, target: reject, condition: { dialect: cel, source: ' ' } } # blank after trimming + - { id: e3, source: check, target: escalate, condition: ' ' } # the shorthand for the same blank source +``` + +> An expression in an evaluated slot needs a non-blank `source`: the expression +> engine evaluates `source` (the canonical persisted form of phase M9.1) and +> cannot evaluate `ast` alone, so an envelope carrying only `ast`, or a `source` +> that is blank after trimming, would validate and register and then fault at +> run time. Write `{ dialect: 'cel', source: '…' }`. + +- **New export `EvaluatedExpressionInputSchema`** (type `EvaluatedExpressionInput`), + the sibling of `ExpressionInputSchema` for an evaluated slot: the bare-string + shorthand still normalizes to `{ dialect: 'cel', source }`, but the string + must be non-blank after trimming, and the envelope arm composes + `EvaluatedExpressionSchema` (`source` required and non-blank) instead of + `ExpressionSchema`. `FlowEdgeSchema.condition` is the first slot to compose + it. An `ast`-only envelope and a blank bare string surface as one + `invalid_union` issue at the slot carrying the sentence above; a blank + `source` inside an envelope surfaces as one `custom` issue at `source`. +- **`ExpressionSchema` / `ExpressionInputSchema` are NOT narrowed.** They remain + the persistence contract (`source` OR `ast`), whose docblock declares that + `ast` becomes required in build output at phase M9.2. When AST-only + evaluation lands, `EvaluatedExpressionSchema` is the one place to relax, and + every evaluated slot follows. +- **`structuralConditionRefusal` no longer admits an `ast`-only envelope** on + either structural condition slot (`config.condition` on a node, + `edge.condition`). #15662's refusal admitted it on purpose through a + `rec.ast !== undefined` clause, because the spec still admitted the shape at + `edge.condition` and refusing it from the consumer side would have decided + #15430's question there; with the edge schema closed, that admission kept the + refusal deliberately holed for a shape the engine cannot run on either slot. + `STRUCTURAL_CONDITION_SHAPE_REFUSAL` now reads "an expression envelope + carrying a string `source`" and says why. Consequence on `config.condition` + (a start node's trigger gate, a decision node's predicate — an open record + with no schema in front of it): an `ast`-only envelope there is refused at + `registerFlow`, reported as a located `error` by `objectstack validate`, and + refused by `evaluateCondition` with the same sentence, instead of answering a + silent `false`. An `ast` BESIDE a string `source` is still admitted + everywhere. The whitespace-only STRING ruling on `config.condition` (#15662: + consistent `false` on both sides) is untouched. +- **Three doors agree, through the spec.** `registerFlow` refuses the flow at + `FlowSchema.parse` (edge) or at its structural pass (`config.condition`); + `objectstack validate` refuses it at its `ObjectStackDefinitionSchema` parse + (edge) or reports the structural refusal (`config.condition`); + `evaluateCondition` refuses the shape a stored flow or a direct caller hands + it. None of them grew a rule of its own. + +**What an author does with a refused edge condition.** An edge condition that +carried only `ast` has no evaluable form under M9.1: author its `source`. A +whitespace-only condition — envelope or bare string — was never a predicate +(the engine answered `false`, so that edge never fired): remove the +`condition` key if the edge was meant to be unconditional, or write the +expression if it was meant to branch. Every edge condition with a +non-blank `source` is unchanged, and nothing is renamed, retired or rewritten — +the refusal itself carries the prescription. + +**A flow ALREADY STORED in `sys_metadata` stops running entirely — the whole +flow, not just the edge.** The paragraph above is the author's remedy, at +`objectstack validate` / `POST /flows`; a stored row has no author in front of +it. Stored flows are deliberately NOT canonicalized by +`applyConversionsToStoredItem` (`spec/src/conversions/stored.ts`, and the same +skip in `metadata/src/loaders/database-loader.ts`'s `rowToData`) — flow-node +conversions need the automation engine's live executor registry, so flows +canonicalize at `registerFlow` instead, which parses through +`canonicalizeStoredFlow` → `FlowSchema.parse`. Each of the three boot paths in +`service-automation/src/plugin.ts` wraps that call in `try`/`catch`, logs one +`warn` naming the flow, and continues. So an edge that used to answer a silent +`false` while the rest of the flow ran now takes the flow down with it: it is +never registered, its trigger is never armed, and the only announcement is that +one warn line — `[Automation] failed to register flow` at boot, +`[Automation] cold-boot flow bind: failed to register flow` at the kernel:ready +bind, `[Automation] flow re-sync: failed to register flow` on a re-sync. That +warn line is also the locator: its `issues[].path` names the offending edge — +`edges[N].condition` — beside the sentence above, so nothing has to be exported +to find it. Author the `source` — or remove the key, if the edge was meant to +be unconditional — and republish. A stack authored in config files has a second +door, `objectstack validate`, which locates the same edge at +`flows.N.edges.N.condition`. Registered as the ADR-0087 D3 semantic entry +`flow-edge-condition-evaluated-slot-source-required`, which carries the same +judgment for a consumer replaying the chain. + +Not touched here: `start.config.condition` has no Zod schema to narrow (the +start node's `config` is an open record); its producer-side gate is the +structural refusal above, which this change tightens but does not type. diff --git a/content/docs/references/api/automation-api.mdx b/content/docs/references/api/automation-api.mdx index 0ddae87017..121cb25ff1 100644 --- a/content/docs/references/api/automation-api.mdx +++ b/content/docs/references/api/automation-api.mdx @@ -144,7 +144,7 @@ const result = AutomationApiErrorCode.parse(data); | **id** | `string` | ✅ | Edge unique ID | | **source** | `string` | ✅ | Source Node ID | | **target** | `string` | ✅ | Target Node ID | -| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. | +| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. An evaluated slot: a bare non-blank CEL string, or an envelope carrying a non-blank `source` — an `ast`-only envelope, and a `source` that is blank after trimming, are refused at authoring because the engine evaluates `source` alone and would otherwise answer a silent `false`. | | **type** | `Enum<'default' \| 'fault' \| 'conditional' \| 'back'>` | optional (default: `"default"`) | Connection type: default (normal flow), fault (error path), conditional (expression-guarded), or back (ADR-0044 declared back-edge — traversed normally at run time, but excluded from DAG cycle validation so a revise/rework loop can re-enter an earlier node) | | **label** | `string` | optional | Label on the connector | | **isDefault** | `boolean` | optional (default: `false`) | BPMN default flow: traverse this edge only when no sibling conditional edge of the same source node matched. Mutually exclusive with `condition`; at most one per source node. | diff --git a/content/docs/references/automation/control-flow.mdx b/content/docs/references/automation/control-flow.mdx index 7f4080f16d..32a319b7c2 100644 --- a/content/docs/references/automation/control-flow.mdx +++ b/content/docs/references/automation/control-flow.mdx @@ -129,7 +129,7 @@ const result = FlowRegionSchema.parse(data); | **id** | `string` | ✅ | Edge unique ID | | **source** | `string` | ✅ | Source Node ID | | **target** | `string` | ✅ | Target Node ID | -| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. | +| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. An evaluated slot: a bare non-blank CEL string, or an envelope carrying a non-blank `source` — an `ast`-only envelope, and a `source` that is blank after trimming, are refused at authoring because the engine evaluates `source` alone and would otherwise answer a silent `false`. | | **type** | `Enum<'default' \| 'fault' \| 'conditional' \| 'back'>` | optional (default: `"default"`) | Connection type: default (normal flow), fault (error path), conditional (expression-guarded), or back (ADR-0044 declared back-edge — traversed normally at run time, but excluded from DAG cycle validation so a revise/rework loop can re-enter an earlier node) | | **label** | `string` | optional | Label on the connector | | **isDefault** | `boolean` | optional (default: `false`) | BPMN default flow: traverse this edge only when no sibling conditional edge of the same source node matched. Mutually exclusive with `condition`; at most one per source node. | @@ -192,7 +192,7 @@ const result = FlowRegionSchema.parse(data); | **id** | `string` | ✅ | Edge unique ID | | **source** | `string` | ✅ | Source Node ID | | **target** | `string` | ✅ | Target Node ID | -| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. | +| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. An evaluated slot: a bare non-blank CEL string, or an envelope carrying a non-blank `source` — an `ast`-only envelope, and a `source` that is blank after trimming, are refused at authoring because the engine evaluates `source` alone and would otherwise answer a silent `false`. | | **type** | `Enum<'default' \| 'fault' \| 'conditional' \| 'back'>` | optional (default: `"default"`) | Connection type: default (normal flow), fault (error path), conditional (expression-guarded), or back (ADR-0044 declared back-edge — traversed normally at run time, but excluded from DAG cycle validation so a revise/rework loop can re-enter an earlier node) | | **label** | `string` | optional | Label on the connector | | **isDefault** | `boolean` | optional (default: `false`) | BPMN default flow: traverse this edge only when no sibling conditional edge of the same source node matched. Mutually exclusive with `condition`; at most one per source node. | diff --git a/content/docs/references/automation/flow.mdx b/content/docs/references/automation/flow.mdx index ab7627bba1..6a2e562a23 100644 --- a/content/docs/references/automation/flow.mdx +++ b/content/docs/references/automation/flow.mdx @@ -84,7 +84,7 @@ const result = FlowSchema.parse(data); | **id** | `string` | ✅ | Edge unique ID | | **source** | `string` | ✅ | Source Node ID | | **target** | `string` | ✅ | Target Node ID | -| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. | +| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. An evaluated slot: a bare non-blank CEL string, or an envelope carrying a non-blank `source` — an `ast`-only envelope, and a `source` that is blank after trimming, are refused at authoring because the engine evaluates `source` alone and would otherwise answer a silent `false`. | | **type** | `Enum<'default' \| 'fault' \| 'conditional' \| 'back'>` | optional (default: `"default"`) | Connection type: default (normal flow), fault (error path), conditional (expression-guarded), or back (ADR-0044 declared back-edge — traversed normally at run time, but excluded from DAG cycle validation so a revise/rework loop can re-enter an earlier node) | | **label** | `string` | optional | Label on the connector | | **isDefault** | `boolean` | optional (default: `false`) | BPMN default flow: traverse this edge only when no sibling conditional edge of the same source node matched. Mutually exclusive with `condition`; at most one per source node. | @@ -122,7 +122,7 @@ const result = FlowSchema.parse(data); | **id** | `string` | ✅ | Edge unique ID | | **source** | `string` | ✅ | Source Node ID | | **target** | `string` | ✅ | Target Node ID | -| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. | +| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL) returning boolean used for branching. An evaluated slot: a bare non-blank CEL string, or an envelope carrying a non-blank `source` — an `ast`-only envelope, and a `source` that is blank after trimming, are refused at authoring because the engine evaluates `source` alone and would otherwise answer a silent `false`. | | **type** | `Enum<'default' \| 'fault' \| 'conditional' \| 'back'>` | optional (default: `"default"`) | Connection type: default (normal flow), fault (error path), conditional (expression-guarded), or back (ADR-0044 declared back-edge — traversed normally at run time, but excluded from DAG cycle validation so a revise/rework loop can re-enter an earlier node) | | **label** | `string` | optional | Label on the connector | | **isDefault** | `boolean` | optional (default: `false`) | BPMN default flow: traverse this edge only when no sibling conditional edge of the same source node matched. Mutually exclusive with `condition`; at most one per source node. | diff --git a/content/docs/references/index.mdx b/content/docs/references/index.mdx index 2ee210f7c9..1baba330da 100644 --- a/content/docs/references/index.mdx +++ b/content/docs/references/index.mdx @@ -1,6 +1,6 @@ --- title: Protocol Reference -description: Every schema published by @objectstack/spec — 1582 schemas across 14 protocol modules +description: Every schema published by @objectstack/spec — 1583 schemas across 14 protocol modules --- {/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} @@ -29,11 +29,11 @@ counts are sums of the rows they head. Regenerate with | [Kernel Protocol](/docs/references/kernel) | 30 | 162 | Plugin lifecycle and manifests, capabilities and security, metadata loading, service registry. | | [QA Protocol](/docs/references/qa) | 1 | 8 | Declarative test suites — scenarios, steps, actions and assertions. | | [Security Protocol](/docs/references/security) | 5 | 30 | Permission sets, row-level security, sharing rules, tenancy posture. | -| [Shared Protocol](/docs/references/shared) | 9 | 28 | Primitives used across every protocol — identifiers, HTTP, expressions, error maps, enums. | +| [Shared Protocol](/docs/references/shared) | 9 | 29 | Primitives used across every protocol — identifiers, HTTP, expressions, error maps, enums. | | [Studio Protocol](/docs/references/studio) | 3 | 35 | Studio designer metadata — the authoring surfaces for the protocols above. | | [System Protocol](/docs/references/system) | 33 | 272 | The runtime environment — logging, jobs, cache, metrics, notifications, i18n and compliance. | | [UI Protocol](/docs/references/ui) | 16 | 153 | Apps, pages, views, dashboards, reports, actions and themes — the ObjectUI layer. | -| **Total** | **198** | **1582** | 14 protocol modules | +| **Total** | **198** | **1583** | 14 protocol modules | --- @@ -286,7 +286,7 @@ Permission sets, row-level security, sharing rules, tenancy posture. ## Shared Protocol -**Source:** `packages/spec/src/shared/` · **Import:** `@objectstack/spec/shared` · **9 pages, 28 schemas** +**Source:** `packages/spec/src/shared/` · **Import:** `@objectstack/spec/shared` · **9 pages, 29 schemas** Primitives used across every protocol — identifiers, HTTP, expressions, error maps, enums. @@ -294,7 +294,7 @@ Primitives used across every protocol — identifiers, HTTP, expressions, error | :--- | :--- | | [`enums.zod.ts`](/docs/references/shared/enums) | `IsolationLevelEnum`, `MutationEventEnum`, `SortDirectionEnum`, `SortItem` | | [`epoch.zod.ts`](/docs/references/shared/epoch) | `EpochMs` | -| [`expression.zod.ts`](/docs/references/shared/expression) | `CronExpressionInput`, `EvaluatedExpression`, `Expression`, `ExpressionDialect`, `ExpressionInput`, `ExpressionMeta`, `Predicate`, `PredicateInput`, `TemplateExpressionInput` | +| [`expression.zod.ts`](/docs/references/shared/expression) | `CronExpressionInput`, `EvaluatedExpression`, `EvaluatedExpressionInput`, `Expression`, `ExpressionDialect`, `ExpressionInput`, `ExpressionMeta`, `Predicate`, `PredicateInput`, `TemplateExpressionInput` | | [`http.zod.ts`](/docs/references/shared/http) | `CorsConfig`, `HttpMethod`, `HttpMethodSubset`, `HttpRequest`, `RateLimitConfig`, `StaticMount` | | [`identifiers.zod.ts`](/docs/references/shared/identifiers) | `MetadataItemName`, `SnakeCaseIdentifier`, `SystemIdentifier` | | [`mapping.zod.ts`](/docs/references/shared/mapping) | `FieldMapping` | diff --git a/content/docs/references/shared/expression.mdx b/content/docs/references/shared/expression.mdx index cad13b7554..b8c4114d5f 100644 --- a/content/docs/references/shared/expression.mdx +++ b/content/docs/references/shared/expression.mdx @@ -57,8 +57,8 @@ See also: content/docs/concepts/north-star.mdx §8 "No private expression DSL" ## TypeScript Usage ```typescript -import { CronExpressionInputSchema, EvaluatedExpressionSchema, ExpressionSchema, ExpressionDialect, ExpressionInputSchema, ExpressionMetaSchema, PredicateSchema, PredicateInputSchema, TemplateExpressionInputSchema } from '@objectstack/spec/shared'; -import type { CronExpressionInput, EvaluatedExpression, Expression, ExpressionDialect, ExpressionInput, ExpressionMeta, Predicate, PredicateInput, TemplateExpressionInput } from '@objectstack/spec/shared'; +import { CronExpressionInputSchema, EvaluatedExpressionSchema, EvaluatedExpressionInputSchema, ExpressionSchema, ExpressionDialect, ExpressionInputSchema, ExpressionMetaSchema, PredicateSchema, PredicateInputSchema, TemplateExpressionInputSchema } from '@objectstack/spec/shared'; +import type { CronExpressionInput, EvaluatedExpression, EvaluatedExpressionInput, Expression, ExpressionDialect, ExpressionInput, ExpressionMeta, Predicate, PredicateInput, TemplateExpressionInput } from '@objectstack/spec/shared'; // Validate data const result = CronExpressionInputSchema.parse(data); @@ -106,6 +106,34 @@ Type: `string` | **meta** | `{ rationale?: string; generatedBy?: string }` | optional | | +--- + +## EvaluatedExpressionInput + +### Union Options + +This schema accepts one of the following structures: + +#### Option 1 + +Type: `string` + +--- + +#### Option 2 + +### Properties + +| Property | Type | Required | Description | +| :--- | :--- | :--- | :--- | +| **dialect** | `Enum<'cel' \| 'cron' \| 'template'>` | ✅ | | +| **source** | `string` | ✅ | | +| **ast** | `any` | optional | | +| **meta** | `{ rationale?: string; generatedBy?: string }` | optional | | + +--- + + --- ## Expression diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 8d844c1613..316c1ef454 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -3896,12 +3896,34 @@ describe('structural condition shape (#15662)', () => { it('refuses an object that is neither text nor an expression', () => { const issues = condIssues({ decisionCondition: { source: 1 } }, "node 'branch'"); expect(issues).toHaveLength(1); - expect(issues[0].message).toContain('neither a string `source` nor an `ast`'); + expect(issues[0].message).toContain('Found an object carrying no string `source`'); // A non-string `source` is what is being refused, so it is never the // attribution. expect(issues[0].source).toBe(''); }); + it('refuses an `ast`-only envelope on a node condition and on an edge — the #15792 admission, revisited by #15807', () => { + // Measured on #15430: the evaluator reads `source` and never `ast`, so this + // envelope answered a silent `false`. On an edge the spec schema refuses it + // one tier earlier under `os validate` (`ObjectStackDefinitionSchema`); on + // this raw-input path, and on `config.condition` everywhere, the structural + // refusal is the gate. + const astOnly = { dialect: 'cel', ast: { kind: 'const', value: true } }; + for (const [opts, site] of [ + [{ decisionCondition: astOnly }, "node 'branch'"], + [{ startCondition: astOnly }, "node 'start'"], + [{ edgeCondition: astOnly }, "edge 'e1'"], + ] as const) { + const issues = condIssues(opts, site); + expect(issues, site).toHaveLength(1); + expect(issues[0].severity).toBe('error'); + expect(issues[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true); + expect(issues[0].message).toContain('Found an object carrying an `ast` but no string `source`'); + } + // CONTROL — an `ast` beside a string `source` is the envelope the engine runs. + expect(condIssues({ edgeCondition: { dialect: 'cel', source: 'record.rating >= 4', ast: { kind: 'const' } } }, "edge 'e1'")).toHaveLength(0); + }); + it('refuses ONCE — the value-reading passes do not re-report it as an empty condition', () => { expect(condIssues({ decisionCondition: 42 }, 'condition')).toHaveLength(1); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 153d7a6524..35def05ef9 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -1194,11 +1194,14 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { * ⚠️ Deliberately NOT `predicateSlotRefusal`, which the declared-slot arm * above uses. A ledger `predicate` slot is declared `z.string()`; neither * structural slot is — `FlowEdgeSchema.condition` is - * `ExpressionInputSchema`, so an envelope is the shape the parse itself - * produces, and a node's `config` is an open `z.record` that passes one - * through verbatim. Both are admitted here; a value that is neither text - * nor an expression is not, because the evaluator reads it as an EMPTY - * condition and answers a silent `false`. + * `EvaluatedExpressionInputSchema` (#15807), so an envelope is the shape + * the parse itself produces, and a node's `config` is an open `z.record` + * that passes one through verbatim. Both are admitted here; a value that + * is neither text nor an envelope carrying a string `source` is not, + * because the evaluator reads it as an EMPTY condition and answers a + * silent `false` — since #15807 that includes an `ast`-only envelope, which + * the edge schema refuses one tier earlier and which this pass is the only + * producer-side gate for on `config.condition`. * * @returns whether the slot was refused, so the caller can skip the * value-reading passes that would otherwise re-report it as an empty one. diff --git a/packages/qa/dogfood/test/expression-conformance.test.ts b/packages/qa/dogfood/test/expression-conformance.test.ts index 02493cbe00..8d1ab01fcc 100644 --- a/packages/qa/dogfood/test/expression-conformance.test.ts +++ b/packages/qa/dogfood/test/expression-conformance.test.ts @@ -79,9 +79,17 @@ const DIALECTS = new Set(['cel', 'cron', 'template', 'js', 'settings-visibility' * population with zero `cron` and zero `template` rows in it, while the spec * declared 12 such positions. Structurally blind, not merely un-updated — which * is why the roster and the rows classifying them landed on one commit. + * + * `EvaluatedExpressionInputSchema` (#15807) is the EVALUATED sibling — the same + * two arms, the string arm non-blank and the envelope arm requiring a non-blank + * `source`. `FlowEdgeSchema.condition` moved onto it, and the very commit that + * moved it listed it here: without this row the edge condition would have + * dropped out of discovery and its `cel-interpret` cover gone STALE — measured, + * on that commit's first CI run (#7327's shape, one more time). */ const EXPRESSION_INPUT_SCHEMAS = [ 'ExpressionInputSchema', + 'EvaluatedExpressionInputSchema', 'SettingsVisibilityInputSchema', 'CronExpressionInputSchema', 'TemplateExpressionInputSchema', diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 7b43a563e8..d67cfbd384 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -8003,13 +8003,19 @@ export class AutomationEngine implements IAutomationService { * * ⚠️ Not `predicateSlotRefusal`, the ledger arm's rule, and the * difference is measured rather than assumed: `FlowEdgeSchema.condition` - * is `ExpressionInputSchema`, whose string arm transforms into - * `{ dialect: 'cel', source }`, so after `FlowSchema.parse` EVERY - * authored edge condition is an envelope — the ledger rule here would - * refuse every conditional edge in every flow. An envelope written at a - * node's `config.condition` is likewise passed through verbatim by the - * open `z.record` and evaluated correctly (#4336). Both are legitimate; - * `structuralConditionRefusal` admits them. + * is `EvaluatedExpressionInputSchema` (#15807), whose string arm + * transforms into `{ dialect: 'cel', source }`, so after + * `FlowSchema.parse` EVERY authored edge condition is an envelope — the + * ledger rule here would refuse every conditional edge in every flow. + * An envelope written at a node's `config.condition` is likewise passed + * through verbatim by the open `z.record` and evaluated correctly + * (#4336). Both are legitimate; `structuralConditionRefusal` admits them + * — an envelope carrying a string `source`, that is. Since #15807 an + * `ast`-only envelope is NOT admitted on either slot: the edge schema + * refuses it one step earlier at `FlowSchema.parse`, and on + * `config.condition` (an open record, so no schema stands in front) this + * pass is the producer-side gate, refusing what `evaluateCondition` + * would otherwise read as an empty condition. * * What it refuses is the value that is neither text nor an expression. * `evaluateCondition` reads `expression?.source ?? ''` and the @@ -9151,9 +9157,12 @@ export class AutomationEngine implements IAutomationService { * What it does NOT refuse is what the constructor admits, and those are * controls, not oversights: every string (a malformed one still earns the * #1491 brace trap or the §1c CEL fault below), absent/`null`, and an - * envelope carrying a string `source` or an `ast` — the `ast`-only arm - * still falls through to `false`, since that population is #15430/#15807's - * and not this ruling's. + * envelope carrying a string `source`. The `ast`-only envelope used to be + * admitted too, and fell through to a silent `false` here — that population + * was #15430/#15807's, not #16038's, and #15807 closed it: the edge schema + * refuses it at authoring, and `structuralConditionRefusal` refuses it on + * both structural slots, so it is refused here as well, through the same + * shared constructor rather than a second rule. */ evaluateCondition(expression: string | { dialect?: string; source?: string; ast?: unknown }, variables: Map): boolean { const shapeRefusal = structuralConditionRefusal(expression); diff --git a/packages/services/service-automation/src/structural-condition-shape.test.ts b/packages/services/service-automation/src/structural-condition-shape.test.ts index 50cad3f784..c81778d1a4 100644 --- a/packages/services/service-automation/src/structural-condition-shape.test.ts +++ b/packages/services/service-automation/src/structural-condition-shape.test.ts @@ -21,10 +21,11 @@ * declared `z.string()`. Neither structural slot is, and the difference was * measured rather than assumed: * - * - `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm - * TRANSFORMS into `{ dialect: 'cel', source }` — so after `FlowSchema.parse` - * every authored edge condition is an envelope. The ledger rule applied here - * would refuse every conditional edge in every flow. + * - `FlowEdgeSchema.condition` is `EvaluatedExpressionInputSchema` (#15807; + * `ExpressionInputSchema` before), whose string arm TRANSFORMS into + * `{ dialect: 'cel', source }` — so after `FlowSchema.parse` every authored + * edge condition is an envelope. The ledger rule applied here would refuse + * every conditional edge in every flow. * - `FlowNodeSchema.config` is an open `z.record`, so an envelope written at * `config.condition` is passed through verbatim by the parse and evaluated * correctly by `evaluateCondition` (#4336's ruling: the dialect is decided @@ -34,6 +35,7 @@ */ import { describe, expect, it, vi } from 'vitest'; import { STRUCTURAL_CONDITION_SHAPE_REFUSAL } from '@objectstack/spec/automation'; +import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec'; import { AutomationEngine } from './engine.js'; @@ -212,6 +214,9 @@ const REFUSED_AT_EVALUATION: Array<[label: string, value: unknown, arm: string]> ['an array', ['a'], 'B'], ['an object that is neither', {}, 'B'], ['an envelope with no source and no ast', { dialect: 'cel' }, 'B'], + // Arm B too, since #15807 — admitted by #15792 on purpose while the spec + // still admitted the shape, and answering `false` off the empty-source arm. + ['an `ast`-only envelope (the #15792 admission, revisited by #15807)', { dialect: 'cel', ast: { kind: 'const', value: true } }, 'B'], ]; describe('#16038 — evaluation refuses the same shapes registration does', () => { @@ -263,11 +268,12 @@ describe('#16038 — evaluation refuses the same shapes registration does', () = expect(evaluate({ dialect: 'cel', source: 'record.rating >= 4' })()).toBe(true); }); - it('an `ast`-only envelope still answers `false` — that population is #15430/#15807', () => { - // `structuralConditionRefusal` admits an `ast`, so this must fall - // through to the empty-source arm exactly as before. If this ever - // throws, the guard swallowed a different card's population. - expect(evaluate({ dialect: 'cel', ast: { kind: 'const' } })()).toBe(false); + it('an `ast` BESIDE a string `source` still evaluates — the engine reads `source`', () => { + // FLIPPED from "an `ast`-only envelope still answers `false`": that + // population was #15430/#15807's, and #15807 closed it (see the + // block below). What stays admitted is the envelope the engine can + // run: a string `source`, with or without an `ast` next to it. + expect(evaluate({ dialect: 'cel', source: 'record.rating >= 4', ast: { kind: 'const' } })()).toBe(true); }); it('a WELL-FORMED non-predicate dialect still answers `false`, not a refusal', () => { @@ -295,3 +301,80 @@ describe('#16038 — evaluation refuses the same shapes registration does', () = }); }); }); + +/** + * #15807 — the edge condition is an EVALUATED slot, and the `ast`-only + * admission #15792 left in `structuralConditionRefusal` is revisited with it. + * + * Measured on #15430 (comment 5550509137): `{ dialect: 'cel', ast: { kind: + * 'const', value: true } }` through `evaluateCondition` answered `false` — the + * engine reads `expression.source ?? ''`, never `ast`, so an `ast`-only + * envelope landed in the empty-source arm and the branch quietly never fired; + * registration said nothing. A whitespace-only `source` was the same seam + * through the other key. + * + * Two doors close it, and they close different populations on purpose: + * + * - `FlowEdgeSchema.condition` now composes `EvaluatedExpressionInputSchema`, + * so on an EDGE both spellings are refused by `FlowSchema.parse` inside + * `registerFlow`, before the structural pass ever sees the edge — with the + * spec's own sentence (`EVALUATED_EXPRESSION_SOURCE_REQUIRED`). + * - `config.condition` is an open record with no schema in front of it, so + * there the structural pass IS the producer-side gate: the + * `rec.ast !== undefined` admission is gone, and an `ast`-only envelope on + * a start node's trigger gate or a decision node's predicate is refused at + * `registerFlow` and at `evaluateCondition` with ONE + * `STRUCTURAL_CONDITION_SHAPE_REFUSAL`. The whitespace-only STRING ruling + * on that slot is untouched (it is #15662's, and consistent on both sides). + */ +describe('#15807 — the edge condition is an evaluated slot; the ast-only admission is gone', () => { + const AST_ONLY = { dialect: 'cel', ast: { kind: 'const', value: true } }; + + describe('on an EDGE — refused by the schema, at registerFlow, with the evaluated-slot sentence', () => { + it('refuses an `ast`-only envelope', () => { + expect(register(flowWith({ edgeCondition: AST_ONLY }))).toThrow(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + // The schema refuses it BEFORE the structural pass runs on the parsed + // flow, so the refusal is the spec's, not the structural sentence. + expect(register(flowWith({ edgeCondition: AST_ONLY }))).not.toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + }); + + it('refuses a `source` that is blank after trimming — envelope and bare-string spellings alike', () => { + expect(register(flowWith({ edgeCondition: { dialect: 'cel', source: ' ' } }))).toThrow(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + expect(register(flowWith({ edgeCondition: ' ' }))).toThrow(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + }); + + it('locates the refusal at the edge the author wrote', () => { + expect(register(flowWith({ edgeCondition: AST_ONLY }))).toThrow(/edges.*0.*condition/s); + }); + + it('CONTROL — an `ast` beside a string `source`, and the bare-string shorthand, still register', () => { + expect(register(flowWith({ edgeCondition: { dialect: 'cel', source: '1 == 1', ast: { kind: 'const' } } }))).not.toThrow(); + expect(register(flowWith({ edgeCondition: '1 == 1' }))).not.toThrow(); + }); + }); + + describe('on `config.condition` — the revisited admission, one refusal at both doors', () => { + const evaluate = (value: unknown) => () => + new AutomationEngine(silentLogger).evaluateCondition(value as never, new Map([['record', { rating: 5 }]])); + + it('refuses an `ast`-only envelope on the decision predicate and on the START trigger gate', () => { + for (const site of ['decisionCondition', 'startCondition'] as const) { + expect(register(flowWith({ [site]: AST_ONLY })), site).toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + expect(register(flowWith({ [site]: AST_ONLY })), site).toThrow('Found an object carrying an `ast` but no string `source`'); + } + }); + + it('refuses it at evaluation with the SAME sentence — no silent `false` any more', () => { + // The measured defect: this answered `false`. It now throws, and the + // error names what was found and what the engine evaluates. + expect(evaluate(AST_ONLY)).toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + expect(evaluate(AST_ONLY)).toThrow('the engine evaluates `source`, never `ast`'); + expect(evaluate({ ast: { kind: 'const', value: true } })).toThrow(STRUCTURAL_CONDITION_SHAPE_REFUSAL); + }); + + it('CONTROL — the whitespace-only STRING ruling on this slot is untouched (#15662)', () => { + expect(register(flowWith({ decisionCondition: ' ' }))).not.toThrow(); + expect(evaluate(' ')()).toBe(false); + }); + }); +}); diff --git a/packages/spec/api-surface/root.json b/packages/spec/api-surface/root.json index 73a4730e7f..dbcd4ad08b 100644 --- a/packages/spec/api-surface/root.json +++ b/packages/spec/api-surface/root.json @@ -63,6 +63,8 @@ "EvalUser (type)", "EvalUserSchema (const)", "EvaluatedExpression (type)", + "EvaluatedExpressionInput (type)", + "EvaluatedExpressionInputSchema (const)", "EvaluatedExpressionParsed (type)", "EvaluatedExpressionSchema (const)", "ExpandViewResult (interface)", diff --git a/packages/spec/api-surface/shared.json b/packages/spec/api-surface/shared.json index f60e60ce9d..9b8e96cdd6 100644 --- a/packages/spec/api-surface/shared.json +++ b/packages/spec/api-surface/shared.json @@ -16,6 +16,8 @@ "EpochMs (const)", "EpochMs (type)", "EvaluatedExpression (type)", + "EvaluatedExpressionInput (type)", + "EvaluatedExpressionInputSchema (const)", "EvaluatedExpressionParsed (type)", "EvaluatedExpressionSchema (const)", "Expression (type)", diff --git a/packages/spec/declaration-map/shared.json b/packages/spec/declaration-map/shared.json index e5193eb953..bdf2db27ec 100644 --- a/packages/spec/declaration-map/shared.json +++ b/packages/spec/declaration-map/shared.json @@ -10,6 +10,8 @@ "CronExpressionInputSchema": "shared/CronExpressionInput", "EpochMs": "shared/EpochMs", "EvaluatedExpression": "shared/EvaluatedExpression", + "EvaluatedExpressionInput": "shared/EvaluatedExpressionInput", + "EvaluatedExpressionInputSchema": "shared/EvaluatedExpressionInput", "EvaluatedExpressionSchema": "shared/EvaluatedExpression", "Expression": "shared/Expression", "ExpressionDialect": "shared/ExpressionDialect", diff --git a/packages/spec/export-origins/root.json b/packages/spec/export-origins/root.json index 75f8348356..f5f450a556 100644 --- a/packages/spec/export-origins/root.json +++ b/packages/spec/export-origins/root.json @@ -63,6 +63,8 @@ "EvalUser": "src/identity/eval-user.zod.ts#EvalUser (type)", "EvalUserSchema": "src/identity/eval-user.zod.ts#EvalUserSchema (const)", "EvaluatedExpression": "src/shared/expression.zod.ts#EvaluatedExpression (type)", + "EvaluatedExpressionInput": "src/shared/expression.zod.ts#EvaluatedExpressionInput (type)", + "EvaluatedExpressionInputSchema": "src/shared/expression.zod.ts#EvaluatedExpressionInputSchema (const)", "EvaluatedExpressionParsed": "src/shared/expression.zod.ts#EvaluatedExpressionParsed (type)", "EvaluatedExpressionSchema": "src/shared/expression.zod.ts#EvaluatedExpressionSchema (const)", "ExpandViewResult": "src/ui/view.zod.ts#ExpandViewResult (interface)", diff --git a/packages/spec/export-origins/shared.json b/packages/spec/export-origins/shared.json index 049d51d3f9..675b46a4cb 100644 --- a/packages/spec/export-origins/shared.json +++ b/packages/spec/export-origins/shared.json @@ -15,6 +15,8 @@ "EXTERNAL_ERROR_HTTP_STATUS": "src/shared/external-errors.ts#EXTERNAL_ERROR_HTTP_STATUS (const)", "EpochMs": "src/shared/epoch.zod.ts#EpochMs (type)", "EvaluatedExpression": "src/shared/expression.zod.ts#EvaluatedExpression (type)", + "EvaluatedExpressionInput": "src/shared/expression.zod.ts#EvaluatedExpressionInput (type)", + "EvaluatedExpressionInputSchema": "src/shared/expression.zod.ts#EvaluatedExpressionInputSchema (const)", "EvaluatedExpressionParsed": "src/shared/expression.zod.ts#EvaluatedExpressionParsed (type)", "EvaluatedExpressionSchema": "src/shared/expression.zod.ts#EvaluatedExpressionSchema (const)", "Expression": "src/shared/expression.zod.ts#Expression (type)", diff --git a/packages/spec/json-schema.manifest/shared.json b/packages/spec/json-schema.manifest/shared.json index 79d7f78dab..68aee49f6a 100644 --- a/packages/spec/json-schema.manifest/shared.json +++ b/packages/spec/json-schema.manifest/shared.json @@ -7,6 +7,7 @@ "shared/CronExpressionInput", "shared/EpochMs", "shared/EvaluatedExpression", + "shared/EvaluatedExpressionInput", "shared/Expression", "shared/ExpressionDialect", "shared/ExpressionInput", diff --git a/packages/spec/src/automation/flow-edge-condition-evaluated.test.ts b/packages/spec/src/automation/flow-edge-condition-evaluated.test.ts new file mode 100644 index 0000000000..90b010e5f0 --- /dev/null +++ b/packages/spec/src/automation/flow-edge-condition-evaluated.test.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `FlowEdgeSchema.condition` is an EVALUATED slot (#15807) — it composes + * `EvaluatedExpressionInputSchema`, so an envelope the engine cannot evaluate + * is refused at authoring instead of answering a silent `false` at run time. + * + * Before this, the slot was `ExpressionInputSchema` — the persistence contract, + * `source` OR `ast` — and `AutomationEngine.evaluateCondition` reads + * `expression.source ?? ''`: an `ast`-only envelope landed in the empty-source + * arm and the branch quietly never fired (measured on #15430: `{ dialect: + * 'cel', ast: { kind: 'const', value: true } }` answered `false`; registration + * said nothing). A whitespace-only `source` was the same seam through the + * other key. + * + * Reproduction pins, one per spelling, each asserting the issue's `code`, + * `path` and message — never `success === false` alone. The controls are the + * persistence contract (`ExpressionInputSchema` / `ExpressionSchema` still + * ACCEPT both shapes, because they were not narrowed) and the accepted + * envelope, byte-identical to what it parsed to before. + */ + +import { describe, expect, it } from 'vitest'; + +import { + EVALUATED_EXPRESSION_SOURCE_REQUIRED, + EvaluatedExpressionInputSchema, + ExpressionInputSchema, + ExpressionSchema, +} from '../shared/expression.zod.js'; +import { ObjectStackDefinitionSchema } from '../stack.zod.js'; +import { FlowEdgeSchema, FlowSchema, type FlowEdge } from './flow.zod.js'; + +const AST_ONLY = { dialect: 'cel', ast: { kind: 'const', value: true } }; +const BLANK_SOURCE = { dialect: 'cel', source: ' ' }; +const GOOD = { dialect: 'cel', source: 'record.amount > 1' }; + +const edge = (condition: unknown) => ({ id: 'e1', source: 'a', target: 'b', condition }); + +function issuesOf(value: unknown) { + const result = FlowEdgeSchema.safeParse(value); + return result.success + ? [] + : result.error.issues.map((i) => ({ code: i.code, path: i.path.map(String).join('.'), message: i.message })); +} + +describe('FlowEdgeSchema.condition — an evaluated slot requires a non-blank `source` (#15807)', () => { + it('REFUSES an `ast`-only envelope: one issue at `condition`, the published sentence', () => { + // Both union arms abort on this shape (the envelope arm's missing `source` + // is an aborting `invalid_type`), so it surfaces as the union's own issue + // at the slot, worded by the union's error map — measured, zod 4.4. + expect(issuesOf(edge(AST_ONLY))).toEqual([ + { code: 'invalid_union', path: 'condition', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }, + ]); + // With no dialect either — the envelope `evaluateCondition` reads as CEL. + expect(issuesOf(edge({ ast: { kind: 'const', value: true } }))).toEqual([ + { code: 'invalid_union', path: 'condition', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }, + ]); + }); + + it.each([ + ['three spaces', ' '], + ['a tab and a newline', '\t\n'], + ['the empty string', ''], + ])('REFUSES an envelope whose `source` is blank after trimming (%s): one issue at `condition.source`', (_label, source) => { + // The envelope arm refuses this one WITHOUT aborting (a `custom` refine), + // so the union reports that arm's issue, at the key the author wrote. + expect(issuesOf(edge({ dialect: 'cel', source }))).toEqual([ + { code: 'custom', path: 'condition.source', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }, + ]); + }); + + it.each([ + ['three spaces', ' '], + ['a tab and a newline', '\t\n'], + ['the empty string', ''], + ])('REFUSES a bare string that is blank after trimming (%s) — the shorthand cannot smuggle the blank `source` in', (_label, source) => { + // Under `ExpressionInputSchema` a whitespace-only string normalized to + // `{ dialect: 'cel', source: ' ' }` — exactly the envelope the evaluated + // rule refuses — so the string arm applies the same trim. + expect(issuesOf(edge(source))).toEqual([ + { code: 'invalid_union', path: 'condition', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }, + ]); + }); + + it('ONE rule — every refused spelling carries the same sentence, and it names what the engine needs and why', () => { + const messages = [AST_ONLY, BLANK_SOURCE, ' '].map((c) => issuesOf(edge(c))[0]!.message); + expect(new Set(messages).size).toBe(1); + expect(messages[0]).toBe(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + expect(EVALUATED_EXPRESSION_SOURCE_REQUIRED).toContain('cannot evaluate `ast` alone'); + }); + + it('does NOT blame `source` for a refusal that is not about it — the map yields to zod\'s default there', () => { + // A number is neither arm's business; a `dialect` outside the enum is the + // enum's refusal. Neither is the evaluated-slot rule, so neither gets its + // sentence (the union's error map returns `undefined` and the default + // stands) — otherwise every malformed edge would prescribe "write a + // `source`" for a defect a `source` would not fix. + for (const value of [42, { dialect: 'js', source: 'x' }]) { + const [issue] = issuesOf(edge(value)); + expect(issue!.code).toBe('invalid_union'); + expect(issue!.path).toBe('condition'); + expect(issue!.message).not.toBe(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + } + }); + + it('ACCEPTS a well-formed envelope unchanged — `source` plus `ast`, `meta`, inner whitespace', () => { + const parsed = FlowEdgeSchema.safeParse(edge(GOOD)); + expect(parsed.success).toBe(true); + expect(parsed.data!.condition).toEqual(GOOD); + // `ast` beside a `source` is fine: the engine still has what it evaluates. + const withAst = { ...GOOD, ast: { kind: 'x' }, meta: { rationale: 'r' } }; + expect(FlowEdgeSchema.safeParse(edge(withAst)).data!.condition).toEqual(withAst); + // The notion of blank is the engine's own (`.trim()`): inner whitespace is authored. + expect(FlowEdgeSchema.safeParse(edge({ dialect: 'cel', source: ' record.amount > 1 ' })).success).toBe(true); + }); + + it('still normalizes the bare-string shorthand to `{ dialect: \'cel\', source }`', () => { + expect(FlowEdgeSchema.safeParse(edge('record.amount > 1')).data!.condition) + .toEqual({ dialect: 'cel', source: 'record.amount > 1' }); + // An absent condition is still "unconditional", not a refused one. + const { condition, ...unconditional } = edge(undefined); + expect(FlowEdgeSchema.safeParse(unconditional).success).toBe(true); + expect(condition).toBeUndefined(); + }); + + it('is refused at the same path through `FlowSchema`, where `registerFlow` parses', () => { + const result = FlowSchema.safeParse({ + name: 'gate_flow', + label: 'Gate Flow', + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'a', type: 'start', label: 'Start', config: { objectName: 'lead', triggerType: 'record-after-update' } }, + { id: 'b', type: 'decision', label: 'Branch', config: {} }, + ], + edges: [edge(AST_ONLY)], + }); + expect(result.success).toBe(false); + expect(result.error!.issues.map((i) => ({ code: i.code, path: i.path.map(String).join('.'), message: i.message }))).toEqual([ + { code: 'invalid_union', path: 'edges.0.condition', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }, + ]); + }); + + it('is refused through `ObjectStackDefinitionSchema` — the parse `objectstack validate` runs before any rule', () => { + // `packages/cli` `validate.ts` step 2 is `ObjectStackDefinitionSchema.safeParse(lowered)` + // and prints `formatZodErrors` on failure, so this is the CLI's reading. + const result = ObjectStackDefinitionSchema.safeParse({ + flows: [{ + name: 'gate_flow', + label: 'Gate Flow', + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'a', type: 'start', label: 'Start', config: { objectName: 'lead', triggerType: 'record-after-update' } }, + { id: 'b', type: 'decision', label: 'Branch', config: {} }, + ], + edges: [edge(AST_ONLY)], + }], + }); + expect(result.success).toBe(false); + const located = result.error!.issues.filter((i) => i.path.map(String).join('.') === 'flows.0.edges.0.condition'); + expect(located.map((i) => ({ code: i.code, message: i.message }))) + .toEqual([{ code: 'invalid_union', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }]); + }); + + it('CONTROL — `ExpressionInputSchema` and `ExpressionSchema`, the persistence contract, still ACCEPT both shapes', () => { + expect(ExpressionInputSchema.safeParse(AST_ONLY).success).toBe(true); + expect(ExpressionInputSchema.safeParse(BLANK_SOURCE).success).toBe(true); + expect(ExpressionInputSchema.safeParse(' ').success).toBe(true); + expect(ExpressionSchema.safeParse(AST_ONLY).success).toBe(true); + expect(ExpressionSchema.safeParse(BLANK_SOURCE).success).toBe(true); + }); + + it('narrows the TYPE too: an `ast`-only edge condition is a compile error before it is a parse error', () => { + const ok: FlowEdge = edge(GOOD) as FlowEdge; + const shorthand: FlowEdge = { id: 'e1', source: 'a', target: 'b', condition: 'record.amount > 1' }; + // @ts-expect-error — `source` is required in an evaluated slot; `ast` alone is not evaluable. + const astOnly: FlowEdge = { id: 'e1', source: 'a', target: 'b', condition: { dialect: 'cel', ast: {} } }; + expect([ok, shorthand, astOnly]).toHaveLength(3); + }); +}); + +describe('EvaluatedExpressionInputSchema — the sibling of ExpressionInputSchema for an evaluated slot (#15807)', () => { + const direct = (value: unknown) => { + const r = EvaluatedExpressionInputSchema.safeParse(value); + return r.success + ? { ok: true as const, data: r.data } + : { ok: false as const, issues: r.error.issues.map((i) => ({ code: i.code, path: i.path.map(String).join('.'), message: i.message })) }; + }; + + it('string arm: non-blank normalizes; blank is refused with the published sentence', () => { + expect(direct('record.amount > 1')).toEqual({ ok: true, data: { dialect: 'cel', source: 'record.amount > 1' } }); + expect(direct(' ')).toEqual({ ok: false, issues: [{ code: 'invalid_union', path: '', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }] }); + }); + + it('envelope arm: composes EvaluatedExpressionSchema — `ast`-only refused, blank `source` refused at `source`', () => { + expect(direct(AST_ONLY)).toEqual({ ok: false, issues: [{ code: 'invalid_union', path: '', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }] }); + expect(direct(BLANK_SOURCE)).toEqual({ ok: false, issues: [{ code: 'custom', path: 'source', message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }] }); + expect(direct(GOOD)).toEqual({ ok: true, data: GOOD }); + // Every declared dialect is still admitted in envelope form — this is the + // untyped input, narrowed on `source` only, not a typed (cron / template) slot. + for (const dialect of ['cron', 'template']) expect(direct({ dialect, source: 'x' }).ok).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 2bce722247..44e5a14224 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.test.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.test.ts @@ -278,12 +278,30 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu expect(structuralConditionRefusal(null)).toBeUndefined(); }); - it('admits an envelope with no dialect, and an ast-only one', () => { + it('admits an envelope with no dialect, and an `ast` BESIDE a string `source`', () => { // `evaluateCondition` already treats an envelope with no dialect as CEL, - // and `ExpressionSchema`'s own refine is `source` OR `ast` — read here, - // not re-derived. + // and reads `source` — an `ast` next to it changes nothing it evaluates. expect(structuralConditionRefusal({ source: 'record.rating >= 4' })).toBeUndefined(); - expect(structuralConditionRefusal({ dialect: 'cel', ast: { kind: 'const' } })).toBeUndefined(); + expect(structuralConditionRefusal({ dialect: 'cel', source: 'record.rating >= 4', ast: { kind: 'const' } })).toBeUndefined(); + }); + + it('REFUSES an `ast`-only envelope — the #15792 admission, revisited by #15807', () => { + // FLIPPED. This admitted `{ dialect: 'cel', ast }` because the spec still + // admitted the shape at `edge.condition` and refusing it here would have + // decided #15430's question from the consumer side. #15807 decided it at + // the producer (`FlowEdgeSchema.condition` composes the evaluated input + // form), and the engine never read `ast` — so an `ast`-only envelope in a + // structural slot is exactly the silent-`false` population this refusal + // exists for, on `config.condition` (still an open record) as on the edge. + for (const value of [{ dialect: 'cel', ast: { kind: 'const', value: true } }, { ast: { kind: 'const' } }]) { + const refusal = structuralConditionRefusal(value); + expect(refusal?.message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true); + expect(refusal?.message).toContain('Found an object carrying an `ast` but no string `source`'); + expect(refusal?.message).toContain('the engine evaluates `source`, never `ast`'); + expect(refusal?.source).toBe(''); + } + // The sentence itself now says why, so the prescription travels with the refusal. + expect(STRUCTURAL_CONDITION_SHAPE_REFUSAL).toContain('an envelope carrying only an `ast` is not evaluable'); }); it('refuses the values measured to register clean and answer a silent false', () => { @@ -299,7 +317,7 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu // `{ source: 1 }` is the one that did not even reach the silent `false`: // it threw a bare `TypeError: exprStr.trim is not a function`. expect(structuralConditionRefusal({ source: 1 })?.message) - .toContain('neither a string `source` nor an `ast`'); + .toContain('Found an object carrying no string `source`'); // An envelope carrying neither — `ExpressionSchema`'s refine rejects it // too, and the evaluator reads it as an empty condition. expect(structuralConditionRefusal({ dialect: 'cel' })).toBeDefined(); diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 5e34032f4a..803fb46195 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -395,22 +395,25 @@ export function predicateSlotRefusal(value: unknown): { message: string; source: * "bare text, an envelope is not authorable" because a ledger `predicate` slot * is *declared* `z.string()`. Neither structural slot is: * - * - `FlowEdgeSchema.condition` is `ExpressionInputSchema`, whose string arm - * **transforms into** `{ dialect: 'cel', source }` — so after - * `FlowSchema.parse` EVERY authored edge condition is an envelope, and the - * ledger arm's rule applied here would refuse every conditional edge in - * every flow. + * - `FlowEdgeSchema.condition` is `EvaluatedExpressionInputSchema` (#15807; + * `ExpressionInputSchema` before that), whose string arm **transforms + * into** `{ dialect: 'cel', source }` — so after `FlowSchema.parse` EVERY + * authored edge condition is an envelope, and the ledger arm's rule applied + * here would refuse every conditional edge in every flow. * - `FlowNodeSchema.config` is an open `z.record`, so an envelope written at * `config.condition` is passed through by the parse verbatim and evaluated * correctly by `evaluateCondition` (both spellings, by #4336's ruling). * * Both shapes are therefore legitimate here and this refusal admits them. What - * it refuses is the third population, which no layer ever admitted on purpose: - * a value that is neither text nor an expression. + * it refuses is the population no layer ever admitted on purpose: a value that + * is neither text nor an envelope the engine can evaluate — and since #15807 + * an envelope the engine can evaluate is one carrying a string `source`; an + * `ast` alone is not one (see {@link structuralConditionRefusal}). */ export const STRUCTURAL_CONDITION_SHAPE_REFUSAL = 'A structural condition (`config.condition` on a node, `edge.condition`) holds either BARE CEL TEXT or an ' - + 'expression envelope — an object carrying a string `source`, or an `ast`. No other shape is authorable there.'; + + 'expression envelope carrying a string `source`. No other shape is authorable there: the engine evaluates ' + + '`source`, so an envelope carrying only an `ast` is not evaluable.'; /** * Why a value sitting in a structural condition slot is not authorable at all — @@ -424,25 +427,45 @@ export const STRUCTURAL_CONDITION_SHAPE_REFUSAL = * is ruled correct, not a defect. * - absent / `null`. "Not authored" is not a malformed predicate; both callers * already return early on it, and this agrees rather than disagreeing. - * - an **expression envelope**: an object carrying a string `source`, or an - * `ast`. That is `ExpressionSchema`'s own rule (`.refine(e => e.source !== - * undefined || e.ast !== undefined)`), read here rather than re-derived, and - * it is the shape `FlowEdgeSchema` produces for every parsed edge condition. + * - an **expression envelope the engine can evaluate**: an object carrying a + * string `source` (an `ast` beside it is fine). That is the evaluated-slot + * rule (`EvaluatedExpressionSchema`, #15430), and it is the shape + * `FlowEdgeSchema` produces for every parsed edge condition since #15807. * `dialect` is not required: an envelope without one is CEL, which is what * `evaluateCondition` already does with it. * + * ## The `ast`-only envelope — admitted until #15807, refused since + * + * This refusal first read `ExpressionSchema`'s own rule (`source` OR `ast`) and + * admitted an envelope carrying only an `ast`, on purpose: the spec still + * admitted that shape at `edge.condition`, and refusing it here would have + * decided #15430's question from the consumer side. #15807 closed that + * question at the producer — `FlowEdgeSchema.condition` now composes the + * evaluated input form, so an `ast`-only edge condition can no longer be + * authored — and this admission went with it: `evaluateCondition` reads + * `source` and never `ast` (`cel-engine.ts` refuses AST-only evaluation), so an + * `ast`-only envelope in either structural slot is exactly the silent-`false` + * population this refusal exists for. Keeping the admission would have left the + * refusal deliberately holed for a shape the schema no longer admits on one + * surface and the engine cannot run on either. When AST-only evaluation lands, + * `EvaluatedExpressionSchema` is the one place to relax, and this clause + * follows it. + * * ## What it refuses, and what that was doing before * - * A number, a boolean, an array, or an object that is neither — `{ source: 1 }`, - * `{ dialect: 'cel' }` with no source and no ast, `{}`. `evaluateCondition` - * reads the source as `expression?.source ?? ''` and the empty-source arm - * returns **`false`**: the "an unauthored branch must not open" rule, applied to - * a value that was very much authored. Measured: `42`, `true` and `['a']` at a - * node's `config.condition` each registered clean, executed `success: true`, and - * said nothing anywhere — on the same key the **start node's trigger gate** is - * read from, so a flow could be silently gated shut forever. `{ source: 1 }` - * did not even get that far: it reached `exprStr.trim()` and threw a bare - * `TypeError` out of the validator. + * A number, a boolean, an array, or an object carrying no string `source` — + * `{ source: 1 }`, `{ dialect: 'cel' }` with no source and no ast, `{}`, and + * `{ dialect: 'cel', ast }`. `evaluateCondition` reads the source as + * `expression?.source ?? ''` and the empty-source arm returns **`false`**: the + * "an unauthored branch must not open" rule, applied to a value that was very + * much authored. Measured: `42`, `true` and `['a']` at a node's + * `config.condition` each registered clean, executed `success: true`, and said + * nothing anywhere — on the same key the **start node's trigger gate** is read + * from, so a flow could be silently gated shut forever; `{ dialect: 'cel', ast: + * { kind: 'const', value: true } }` through `evaluateCondition` answered `false` + * (#15430's seat, on #15662's measurement). `{ source: 1 }` did not even get + * that far: it reached `exprStr.trim()` and threw a bare `TypeError` out of the + * validator. * * Refusing at the producer is the contract-first half: the flow does not * register and `objectstack validate` locates it, rather than the reject set of @@ -457,13 +480,15 @@ export function structuralConditionRefusal( if (value == null) return undefined; if (typeof value === 'string') return undefined; if (typeof value === 'object' && !Array.isArray(value)) { - const rec = value as { source?: unknown; ast?: unknown }; - if (typeof rec.source === 'string' || rec.ast !== undefined) return undefined; + const rec = value as { source?: unknown }; + if (typeof rec.source === 'string') return undefined; } const found = Array.isArray(value) ? 'an array' : typeof value === 'object' - ? 'an object carrying neither a string `source` nor an `ast`' + ? (value as { ast?: unknown }).ast !== undefined + ? 'an object carrying an `ast` but no string `source` — the engine evaluates `source`, never `ast`' + : 'an object carrying no string `source`' : `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. A non-string diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index b4fb9995ef..968be831f2 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { ProtectionSchema } from '../shared/protection.zod'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; -import { ExpressionInputSchema } from '../shared/expression.zod'; +import { EvaluatedExpressionInputSchema } from '../shared/expression.zod'; /** * Flow Node Types — **built-in seed set** (ADR-0018). @@ -563,8 +563,27 @@ export const FlowEdgeSchema = lazySchema(() => strictObject( source: z.string().describe('Source Node ID'), target: z.string().describe('Target Node ID'), - /** Condition for this path (only for decision/branch nodes) */ - condition: ExpressionInputSchema.optional().describe('Predicate (CEL) returning boolean used for branching.'), + /** + * Condition for this path (only for decision/branch nodes). + * + * An EVALUATED slot (#15807): `AutomationEngine.evaluateCondition` runs it + * at every traversal, reading `source` and nothing else, so it composes + * `EvaluatedExpressionInputSchema` rather than `ExpressionInputSchema` — + * the bare-string shorthand still normalizes to `{ dialect: 'cel', source }`, + * but the string must be non-blank after trimming and an envelope must carry + * a non-blank `source`. Under the persistence contract an `ast`-only + * envelope, and a whitespace-only `source`, both parsed here, registered, + * passed `objectstack validate`, and then landed in the evaluator's + * empty-source arm and answered a SILENT `false`: a branch that quietly + * never fired. Both are refused at authoring now, with + * `EVALUATED_EXPRESSION_SOURCE_REQUIRED`. + */ + condition: EvaluatedExpressionInputSchema.optional().describe( + 'Predicate (CEL) returning boolean used for branching. An evaluated slot: a bare non-blank CEL string, or an ' + + 'envelope carrying a non-blank `source` — an `ast`-only envelope, and a `source` that is blank after ' + + 'trimming, are refused at authoring because the engine evaluates `source` alone and would otherwise answer ' + + 'a silent `false`.', + ), type: z.enum(['default', 'fault', 'conditional', 'back']) .default('default') diff --git a/packages/spec/src/index.ts b/packages/spec/src/index.ts index bd6f84650c..f4adc1e83b 100644 --- a/packages/spec/src/index.ts +++ b/packages/spec/src/index.ts @@ -257,6 +257,7 @@ export { EVALUATED_EXPRESSION_SOURCE_REQUIRED, EvaluatedExpressionSchema, ExpressionInputSchema, + EvaluatedExpressionInputSchema, CronExpressionInputSchema, TemplateExpressionInputSchema, PredicateSchema, @@ -274,6 +275,7 @@ export type { EvaluatedExpressionParsed, ExpressionMeta, ExpressionInput, + EvaluatedExpressionInput, Predicate, PredicateInput, } from './shared/expression.zod'; diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-edge-condition-evaluated-slot-source-required.ts b/packages/spec/src/migrations/entries/semantic/18.flow-edge-condition-evaluated-slot-source-required.ts new file mode 100644 index 0000000000..af181ced49 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-edge-condition-evaluated-slot-source-required.ts @@ -0,0 +1,76 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// 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-edge-condition-evaluated-slot-source-required', + surface: + 'a flow edge predicate — edges[].condition on FlowEdgeSchema, the branch predicate ' + + 'AutomationEngine.evaluateCondition runs at every traversal — authored either as an ' + + 'expression envelope carrying only ast ({ dialect: \'cel\', ast: … } with no source), or ' + + 'with a source that is blank after trimming, through the envelope key ({ dialect: \'cel\', ' + + 'source: \' \' }) or the bare-string shorthand for it (condition: \' \'). Reachable ' + + 'wherever a flow is authored or stored: defineStack({ flows }) sources, an exported stack ' + + 'passed to objectstack validate, a POST /flows body, and a flow row already sitting in ' + + 'sys_metadata', + replacement: + 'a non-blank `source` — `{ dialect: \'cel\', source: \'record.amount > 10\' }`, or the bare ' + + 'string `\'record.amount > 10\'` — if the edge was meant to branch; or REMOVE the ' + + '`condition` key entirely if it was meant to be unconditional. ⚠️ Those two are not ' + + 'interchangeable, and the choice is the judgment this entry delegates: a refused condition ' + + 'evaluated to a silent `false`, so the edge NEVER fired, while an absent `condition` is an ' + + 'unconditional edge that ALWAYS fires. Deleting the key to clear the refusal inverts the ' + + 'edge rather than preserving it. An `ast` BESIDE a string `source` is untouched and stays ' + + 'admitted everywhere', + reason: + 'Card #15807 (the #15430 / #15662 lineage): `FlowEdgeSchema.condition` now composes ' + + '`EvaluatedExpressionInputSchema` instead of `ExpressionInputSchema`, so an evaluated slot ' + + 'is held to what the engine can actually run. The engine reads `source` alone ' + + '(`cel-engine.ts` `evaluate`: "AST-only evaluation not yet supported; persist `source`"), ' + + 'so both refused spellings landed in its empty-source arm and answered a SILENT `false` on ' + + 'every release that carried them — they parsed, registered, passed `objectstack validate`, ' + + 'and then produced a branch that quietly never fired (measured on #15430, comment ' + + '5550509137). The refusal is one rule with one sentence, ' + + '`EVALUATED_EXPRESSION_SOURCE_REQUIRED`. ' + + '⚠️ No D2 conversion is possible, and this is exactly why the change needs a D3 entry ' + + 'rather than none. An `ast`-only envelope carries no `source` to derive one from — ' + + 'lowering an AST to surface syntax is the compiler direction the platform does not run — ' + + 'and dropping a blank `condition` would flip the edge from never-fires to ALWAYS-fires, ' + + 'which is the platform guessing which of two different flows the author meant. ' + + '⚠️ And the consequence for a flow ALREADY STORED is wider than the edge, which is the ' + + 'part no author-time prescription reaches. `applyConversionsToStoredItem` is deliberately ' + + 'not applied to `flow` (`spec/src/conversions/stored.ts`, and the same skip in ' + + '`metadata/src/loaders/database-loader.ts` `rowToData`) because flow-node conversions need ' + + 'the automation engine\'s live executor registry; flows canonicalize at `registerFlow` ' + + 'instead, which parses through `canonicalizeStoredFlow` → `FlowSchema.parse`. Each of the ' + + 'three boot paths in `service-automation/src/plugin.ts` wraps that call in try/catch, logs ' + + 'one `warn` naming the flow, and CONTINUES — so a stored `sys_metadata` flow with such an ' + + 'edge is no longer registered at all: its trigger is never armed and the WHOLE flow stops ' + + 'running, not just the branch, announced only by that warn line. A repo-wide census at ' + + '`ae19f5edb` (examples/, packages/, content/, skills/) found zero edge conditions of either ' + + 'spelling against a lit control, so there is nothing in THIS repository to rewrite — a ' + + 'repo reading, which is why the notification is registered here rather than skipped. ' + + 'ADR-0087, ADR-0032.', + acceptanceCriteria: + 'Grep every authored `edges[].condition` — `defineStack({ flows })` sources, exported stacks, ' + + '`POST /flows` bodies — and every flow row in `sys_metadata`, for an envelope with no ' + + '`source` key and for a `source` (or bare string) that is empty after trimming. For each ' + + 'hit decide, per the `replacement` note, whether the edge was meant to branch (author the ' + + '`source`) or to be unconditional (remove the key) — do not default to removal. Two proofs, ' + + 'and the second is the one that matters for stored rows. (1) For a stack authored in config ' + + 'files, `objectstack validate` is clean: it locates each offender at ' + + '`flows.N.edges.N.condition` with the `EVALUATED_EXPRESSION_SOURCE_REQUIRED` sentence, and ' + + 'an `ast`-only envelope is also reported by the lint path as ' + + '`STRUCTURAL_CONDITION_SHAPE_REFUSAL`. There is no CLI verb that lowers a stored row back ' + + 'into a config file, so this proof does not reach a flow that exists only in ' + + '`sys_metadata`. (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`), and ' + + 'its trigger is armed. That warn line IS the locator for a stored row: its `issues[].path` ' + + 'names the offending edge as `edges[N].condition`. A flow that boots without that warn is ' + + 'unaffected; every edge ' + + 'condition carrying a non-blank `source` parses byte-identically to before.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index ab1cd97668..582f91440e 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -7560,6 +7560,78 @@ const step18: MigrationStep = { + 'door, 400 after), so re-check what the surface was supposed to show rather than ' + 'assuming the old result set was correct.', }, + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code + // span already, and a nested backtick would close it. + { + id: 'flow-edge-condition-evaluated-slot-source-required', + surface: + 'a flow edge predicate — edges[].condition on FlowEdgeSchema, the branch predicate ' + + 'AutomationEngine.evaluateCondition runs at every traversal — authored either as an ' + + 'expression envelope carrying only ast ({ dialect: \'cel\', ast: … } with no source), or ' + + 'with a source that is blank after trimming, through the envelope key ({ dialect: \'cel\', ' + + 'source: \' \' }) or the bare-string shorthand for it (condition: \' \'). Reachable ' + + 'wherever a flow is authored or stored: defineStack({ flows }) sources, an exported stack ' + + 'passed to objectstack validate, a POST /flows body, and a flow row already sitting in ' + + 'sys_metadata', + replacement: + 'a non-blank `source` — `{ dialect: \'cel\', source: \'record.amount > 10\' }`, or the bare ' + + 'string `\'record.amount > 10\'` — if the edge was meant to branch; or REMOVE the ' + + '`condition` key entirely if it was meant to be unconditional. ⚠️ Those two are not ' + + 'interchangeable, and the choice is the judgment this entry delegates: a refused condition ' + + 'evaluated to a silent `false`, so the edge NEVER fired, while an absent `condition` is an ' + + 'unconditional edge that ALWAYS fires. Deleting the key to clear the refusal inverts the ' + + 'edge rather than preserving it. An `ast` BESIDE a string `source` is untouched and stays ' + + 'admitted everywhere', + reason: + 'Card #15807 (the #15430 / #15662 lineage): `FlowEdgeSchema.condition` now composes ' + + '`EvaluatedExpressionInputSchema` instead of `ExpressionInputSchema`, so an evaluated slot ' + + 'is held to what the engine can actually run. The engine reads `source` alone ' + + '(`cel-engine.ts` `evaluate`: "AST-only evaluation not yet supported; persist `source`"), ' + + 'so both refused spellings landed in its empty-source arm and answered a SILENT `false` on ' + + 'every release that carried them — they parsed, registered, passed `objectstack validate`, ' + + 'and then produced a branch that quietly never fired (measured on #15430, comment ' + + '5550509137). The refusal is one rule with one sentence, ' + + '`EVALUATED_EXPRESSION_SOURCE_REQUIRED`. ' + + '⚠️ No D2 conversion is possible, and this is exactly why the change needs a D3 entry ' + + 'rather than none. An `ast`-only envelope carries no `source` to derive one from — ' + + 'lowering an AST to surface syntax is the compiler direction the platform does not run — ' + + 'and dropping a blank `condition` would flip the edge from never-fires to ALWAYS-fires, ' + + 'which is the platform guessing which of two different flows the author meant. ' + + '⚠️ And the consequence for a flow ALREADY STORED is wider than the edge, which is the ' + + 'part no author-time prescription reaches. `applyConversionsToStoredItem` is deliberately ' + + 'not applied to `flow` (`spec/src/conversions/stored.ts`, and the same skip in ' + + '`metadata/src/loaders/database-loader.ts` `rowToData`) because flow-node conversions need ' + + 'the automation engine\'s live executor registry; flows canonicalize at `registerFlow` ' + + 'instead, which parses through `canonicalizeStoredFlow` → `FlowSchema.parse`. Each of the ' + + 'three boot paths in `service-automation/src/plugin.ts` wraps that call in try/catch, logs ' + + 'one `warn` naming the flow, and CONTINUES — so a stored `sys_metadata` flow with such an ' + + 'edge is no longer registered at all: its trigger is never armed and the WHOLE flow stops ' + + 'running, not just the branch, announced only by that warn line. A repo-wide census at ' + + '`ae19f5edb` (examples/, packages/, content/, skills/) found zero edge conditions of either ' + + 'spelling against a lit control, so there is nothing in THIS repository to rewrite — a ' + + 'repo reading, which is why the notification is registered here rather than skipped. ' + + 'ADR-0087, ADR-0032.', + acceptanceCriteria: + 'Grep every authored `edges[].condition` — `defineStack({ flows })` sources, exported stacks, ' + + '`POST /flows` bodies — and every flow row in `sys_metadata`, for an envelope with no ' + + '`source` key and for a `source` (or bare string) that is empty after trimming. For each ' + + 'hit decide, per the `replacement` note, whether the edge was meant to branch (author the ' + + '`source`) or to be unconditional (remove the key) — do not default to removal. Two proofs, ' + + 'and the second is the one that matters for stored rows. (1) For a stack authored in config ' + + 'files, `objectstack validate` is clean: it locates each offender at ' + + '`flows.N.edges.N.condition` with the `EVALUATED_EXPRESSION_SOURCE_REQUIRED` sentence, and ' + + 'an `ast`-only envelope is also reported by the lint path as ' + + '`STRUCTURAL_CONDITION_SHAPE_REFUSAL`. There is no CLI verb that lowers a stored row back ' + + 'into a config file, so this proof does not reach a flow that exists only in ' + + '`sys_metadata`. (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`), and ' + + 'its trigger is armed. That warn line IS the locator for a stored row: its `issues[].path` ' + + 'names the offending edge as `edges[N].condition`. A flow that boots without that warn is ' + + 'unaffected; every edge ' + + 'condition carrying a non-blank `source` parses byte-identically to before.', + }, { id: 'hot-reload-inert-state-strategies-retired', surface: diff --git a/packages/spec/src/shared/expression.zod.ts b/packages/spec/src/shared/expression.zod.ts index 78a47d454c..d3ddffa7bd 100644 --- a/packages/spec/src/shared/expression.zod.ts +++ b/packages/spec/src/shared/expression.zod.ts @@ -149,6 +149,13 @@ export const EVALUATED_EXPRESSION_SOURCE_REQUIRED = * shape, and narrows the TYPE too (`source: string`) — the same move * `AssignmentExpressionValueSchema` makes for `dialect`, so an `ast`-only * envelope is a compile error before it is a parse error. + * + * That promise is scoped to a DIRECT caller of this schema (#15807). Composed + * through {@link EvaluatedExpressionInputSchema}, only the blank-`source` + * envelope still surfaces from here, at `source`; an `ast`-only envelope and a + * blank bare string abort BOTH arms of that union and surface as one + * `invalid_union` at the SLOT instead, carrying this same sentence from + * `evaluatedExpressionInputRefusal` — see that map's docblock for why. */ export const EvaluatedExpressionSchema = ExpressionSchema.safeExtend({ /** @@ -174,6 +181,72 @@ export const ExpressionInputSchema = z.union([ ]); export type ExpressionInput = z.input; +/** + * Why an EVALUATED input slot refused a value, in the union's own words — the + * one place the sentence for a refused union input lives (#15807). + * + * A union reports the one arm that did not abort, else `invalid_union` at the + * slot (zod 4.4, measured): here BOTH arms abort on the two shapes this slot + * exists to refuse — the string arm's refine is inside a pipe whose transform + * aborts the arm, and the envelope arm's missing `source` is an aborting + * `invalid_type` — so an `ast`-only envelope and a blank bare string each + * surface as ONE `invalid_union` at the slot, and this map is where its message + * comes from. A blank `source` INSIDE an envelope is the one shape the envelope + * arm refuses without aborting (a `custom` refine), so that one surfaces from + * the arm itself, at `source`, and never reaches this map. + * + * It answers {@link EVALUATED_EXPRESSION_SOURCE_REQUIRED} exactly when the + * refusal IS the evaluated-slot rule — a string input (only a blank one gets + * here), or an object input carrying no string `source` — and `undefined` for + * everything else (a number, an envelope naming a dialect outside the enum), + * which lets zod's default message stand rather than blaming `source` for a + * refusal that is not about it. + */ +function evaluatedExpressionInputRefusal(input: unknown): string | undefined { + if (typeof input === 'string') return EVALUATED_EXPRESSION_SOURCE_REQUIRED; + if (input && typeof input === 'object' && !Array.isArray(input) + && typeof (input as { source?: unknown }).source !== 'string') { + return EVALUATED_EXPRESSION_SOURCE_REQUIRED; + } + return undefined; +} + +/** + * Author-time input shape of an EVALUATED slot — the sibling of + * {@link ExpressionInputSchema} for a slot whose value the expression engine + * runs (#15807; the rule is {@link EvaluatedExpressionSchema}'s, #15430). + * + * The same two arms as `ExpressionInputSchema`, each held to the evaluated + * rule: a bare string is still shorthand for `{ dialect: 'cel', source }`, but + * it must be non-blank after trimming (a blank string would normalize to an + * envelope carrying exactly the blank `source` the envelope arm refuses — one + * seam, two keys, one rule); and the envelope arm composes + * `EvaluatedExpressionSchema` instead of `ExpressionSchema`, so `source` is + * required and non-blank there too. `ExpressionSchema` / `ExpressionInputSchema` + * are NOT narrowed: they remain the persistence contract (`source` OR `ast`). + * + * The first slot to compose it is `FlowEdgeSchema.condition`, the branch + * predicate `evaluateCondition` runs: an `ast`-only envelope authored there + * used to parse, register, pass `objectstack validate`, and then land in the + * evaluator's empty-source arm and answer a SILENT `false` — a branch that + * quietly never fires — and a whitespace-only `source` did the same through + * the other key. Both are refused at authoring now, with the published + * sentence: an `ast`-only envelope and a blank bare string as one + * `invalid_union` at the slot (see {@link evaluatedExpressionInputRefusal} for + * why), a blank `source` inside an envelope as one `custom` issue at `source`. + * + * The string arm's transform returns the narrowed `{ dialect: 'cel', source }` + * as an `EvaluatedExpression`, so the parsed value of an evaluated slot stays + * assignable to its own input type — the same move the typed arms make. + */ +export const EvaluatedExpressionInputSchema = z.union([ + z.string() + .refine((source) => source.trim().length > 0, { message: EVALUATED_EXPRESSION_SOURCE_REQUIRED }) + .transform((source): EvaluatedExpression => ({ dialect: 'cel', source })), + EvaluatedExpressionSchema, +], { error: (issue) => evaluatedExpressionInputRefusal(issue.input) }); +export type EvaluatedExpressionInput = z.input; + /** * The dialects that have a TYPED input schema below. On a typed slot the bare * string is shorthand for this dialect, and the envelope arm accepts this