From c4ee703f6a6873898573aced46537eecd60df2b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:11:37 +0000 Subject: [PATCH 1/7] wip(spec): refuse a blank string in a ledger predicate slot at FlowSchema.parse and in predicateSlotRefusal Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- .../automation/flow-node-expression-paths.ts | 140 +++++++++++++----- packages/spec/src/automation/flow.zod.ts | 71 ++++++++- 2 files changed, 172 insertions(+), 39 deletions(-) diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 8ab4f951551..6da92e96af3 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -72,6 +72,10 @@ * implements that dialect are checked. */ +// The repo's one notion of "blank" (`source.trim()`), shared with the evaluated +// slots — never a second hand-written one here. +import { NON_BLANK_STRING } from '../shared/refinement-projection'; + /** * The dialect a declared expression slot takes — and therefore what, if * anything, can validate it today. @@ -279,20 +283,28 @@ export function isExpressionEnvelopeShaped(value: unknown): value is { dialect: * * What counts as "an authored expression" depends on the role: * - * - `predicate`: the slot IS the expression, so a non-empty string is emitted. - * Absent and `null` are skipped — "not authored" is not a malformed - * expression — and so is a whitespace-only string, which both validators and - * the evaluator already agree means the same thing. A **non-string** is - * emitted too (#15572), for the consumer to refuse through + * - `predicate`: the slot IS the expression, so every string is emitted — + * including one that is blank after trimming (#17493). Absent and `null` + * are skipped: "not authored" is not a malformed expression. A **non-string** + * is emitted too (#15572), for the consumer to refuse through * {@link predicateSlotRefusal}: it used to be skipped as "a type violation * for the schema pass to report", and for a schemaless node type there is no * schema pass — `decision` publishes no descriptor `configSchema`, and * `DecisionConditionSchema` is never parsed against a node's open * `z.record` config — so nothing reported it and the value ran anyway. - * - `flow-template`: same string rule, minus the refusal. No validator - * implements that dialect (see {@link FlowNodeExpressionRole}), so emitting a - * non-string there would hand every consumer a finding none of them can - * judge. + * + * ⚠️ The blank string used to be SKIPPED here as "not authored", on the + * ground that both validators and the evaluator treated it the same way. + * That ground was true and was ruled insufficient (#17493, ruling A): a + * blank predicate is an author who meant to write a rule, and a parser and + * an evaluator agreeing to ignore it only proves the platform did not crash. + * It is now emitted so the consumer can refuse it, by the same + * {@link predicateSlotRefusal} every other unauthorable value meets. + * - `flow-template`: the pre-#17493 string rule — a non-blank string only — + * minus the refusal. No validator implements that dialect (see + * {@link FlowNodeExpressionRole}), so emitting a blank or a non-string there + * would hand every consumer a finding none of them can judge; that ruling + * reached the `predicate` slots and nothing else. * - `value`: the slot holds a VALUE that may be spelled as an expression, so * only envelope-shaped objects ({@link isExpressionEnvelopeShaped}) are * emitted. A string there is `{token}` interpolation — the `flow-template` @@ -312,7 +324,10 @@ export function resolveFlowNodeExpressions( if (entry.role === 'value') { if (isExpressionEnvelopeShaped(value)) out.push({ entry, path, value }); } else if (typeof value === 'string') { - if (value.trim()) out.push({ entry, path, value }); + // #17493 — a BLANK string in a `predicate` slot is emitted, for the + // consumer to refuse (see `predicateSlotRefusal`); a `flow-template` + // slot keeps skipping it. + if (entry.role === 'predicate' || NON_BLANK_STRING(value)) out.push({ entry, path, value }); } else if (entry.role === 'predicate' && value != null) { // #15572 — a non-string in a predicate slot. Emitted so a consumer can // REFUSE it (see `predicateSlotRefusal`), never so it can be parsed. @@ -328,19 +343,37 @@ export function resolveFlowNodeExpressions( * words however the value is wrong, so an author (or an agent reading the * failure) learns the rule before the detail. Mirrors * `ASSIGNMENT_VALUE_ENVELOPE_REFUSAL`, the `value` role's equivalent. + * + * #17493 widened the sentence rather than adding a second one: a string that is + * blank after trimming is refused by the same rule, so the sentence names it. + * Every refusal of a `predicate` slot — at `FlowSchema.parse`, + * `AutomationEngine.registerFlow` and `objectstack validate` — leads with it. */ export const PREDICATE_SLOT_STRING_REFUSAL = - 'A predicate slot holds BARE CEL TEXT — it is declared `z.string()` — so an expression envelope, or any other ' - + 'non-string, is not authorable there.'; + 'A predicate slot holds BARE CEL TEXT that states a rule — it is declared `z.string()` — so an expression ' + + 'envelope, any other non-string, or a string that is blank after trimming is not authorable there.'; /** * Why a value sitting in a `predicate`-role slot is not authorable at all — - * the SINGLE notion both consumers apply, derived once (#15572). - * - * `undefined` for every string: what a string *says* is judged by - * `validateExpression('predicate', …)`, unchanged, and a whitespace-only string - * never reaches here (the resolver treats it as "not authored", the same way - * the evaluator does — deliberately, and not a defect on either side). + * the SINGLE notion every door applies, derived once (#15572, #17493). + * + * `undefined` for every NON-BLANK string: what such a string *says* is judged + * by `validateExpression('predicate', …)`, unchanged. + * + * ## The blank string — refused since #17493 + * + * A string that is blank after trimming (`''`, `' '`, `'\t\n'`) is refused + * here, with the sentence above. This function used to return `undefined` for + * it too, and its docblock called that deliberate: the resolver skipped the + * blank as "not authored" and the evaluator answered it the same way, so the + * two sides agreed and neither was ruled a defect. #17493's ruling A + * (5651023407) superseded that ground — self-consistency between parser and + * evaluator is not a defence when the author's intent is silently dropped: + * a `decision` branch whose `expression` is blank is never taken, and nothing + * said so at any layer. The ruling is the third of one rule, after #17322 (the + * structural `config.condition`) and #15811 (the evaluated `source`), so the + * blank is refused by the SAME notion of blank those use (`NON_BLANK_STRING`, + * i.e. `trim()`), under this slot's own sentence. * * ## Why this is a refusal and not a parse * @@ -359,10 +392,22 @@ export const PREDICATE_SLOT_STRING_REFUSAL = * on a predicate no validator ever read. * * @returns the refusal and the source to attribute it to, or `undefined` when - * the value is a string and therefore this function's business is done. + * the value is a non-blank string and therefore this function's business is + * done. */ export function predicateSlotRefusal(value: unknown): { message: string; source: string } | undefined { - if (typeof value === 'string') return undefined; + if (typeof value === 'string') { + if (NON_BLANK_STRING(value)) return undefined; + return { + message: + `${PREDICATE_SLOT_STRING_REFUSAL} Found a string that is blank after trimming, which states no rule. ` + + 'Write the predicate the branch or field was meant to test (e.g. `record.rating >= 4`), or remove it: ' + + 'on a screen field, drop the `visibleWhen` key to show the field unconditionally; on a decision ' + + 'branch, `expression` is required, so drop that branch from `conditions` — a branch whose predicate ' + + 'is blank is never taken.', + source: value, + }; + } const envelope = isExpressionEnvelopeShaped(value); const found = envelope ? 'an expression envelope (an object naming a `dialect`)' @@ -433,8 +478,14 @@ export const STRUCTURAL_CONDITION_SHAPE_REFUSAL = * `FlowSchema.parse`. #17322 then ruled on the disagreement that left * (一个操作两个实现且行为不一致 ⇒ 带治理的一侧胜出,另一侧改绑) and rebound the node * door at `AutomationEngine.registerFlow`; #17495 followed at - * `objectstack validate`. A blank structural condition is a defect today, - * refused at all three doors. + * `objectstack validate`. A blank structural condition is a defect today: + * on an edge it is refused at `FlowSchema.parse` (and so at the two doors + * that parse first), and on a node at `registerFlow` and + * `objectstack validate`. ⚠️ Not at `FlowSchema.parse` on a node — its + * `config` is an open record the flow parse passes a blank + * `config.condition` through, which is why this sentence used to read "all + * three doors" and no longer does (#17493 measured it while giving the + * sibling `predicate` slots below a flow-parse refusal of their own). * * It is refused there by the EVALUATED-SLOT rule, not by this one. Both * consumers ask `EvaluatedExpressionInputSchema` — the edge door's own @@ -471,22 +522,35 @@ export const STRUCTURAL_CONDITION_SHAPE_REFUSAL = * `EvaluatedExpressionSchema` is the one place to relax, and this clause * follows it. * - * ## The sibling `predicate` slots — an OPEN question, not answered here - * - * The blank rule reached the two STRUCTURAL slots only. The ledger `predicate` - * slots — `config.conditions[].expression`, a `decision` node's branch list, and - * `screen.fields[].visibleWhen` — are judged by {@link predicateSlotRefusal}, - * not by this function, and they still ADMIT a whitespace-only string: - * registration takes it and `evaluateCondition` answers `false`, the same silent - * dead branch #17322 closed one slot over. Recorded here rather than fixed, - * because it is a RULING and not a refactor: #15572 pinned that admission as - * correct on the very ground #15807 removed — that the blank is treated the same - * way on both sides — so narrowing those slots re-judges a pin and moves a - * published accept-set. #17493 carries the question (does the ledger predicate - * slot follow the structural one?) and it is open at the time of writing. ⛔ Do - * not answer it by widening this refusal: those slots do not pass through this - * door, and a second notion of "blank" is what the shared refusal exists to - * prevent. + * ## The sibling `predicate` slots — ruled, and refused by THEIR rule, not this one + * + * The blank rule first reached the two STRUCTURAL slots only. The ledger + * `predicate` slots — `config.conditions[].expression`, a `decision` node's + * branch list, and `screen.fields[].visibleWhen` — are judged by + * {@link predicateSlotRefusal}, not by this function, and they went on + * admitting a blank string after #17322: registration took it, and a `decision` + * branch carrying it was never taken — the same silent dead branch #17322 + * closed one slot over. + * + * This paragraph used to record that as an OPEN question, and said why it was + * not fixed here: it was a RULING and not a refactor, because #15572 had pinned + * that admission as correct on the ground that the blank was treated the same + * way on both sides (the resolver skipped it, the evaluator answered it), so + * narrowing those slots re-judged a pin and moved a published accept set. + * #17493 carried the question — does the ledger predicate slot follow the + * structural one? — and its ruling A (5651023407) answered YES, on a ground + * wider than this card: self-consistency between parser and evaluator is not a + * defence when the author's intent is silently dropped. What changed this + * paragraph is that ruling, not a new measurement — the two sides still agree + * with each other, and that agreement no longer suffices. + * + * So both slots now REFUSE a blank string, at `FlowSchema.parse`, + * `AutomationEngine.registerFlow` and `objectstack validate`, leading with + * {@link PREDICATE_SLOT_STRING_REFUSAL}. ⛔ Still not by widening THIS + * refusal: those slots do not pass through this door, a string is still a + * well-shaped structural condition, and ONE notion of "blank" — the shared + * `NON_BLANK_STRING`, which the evaluated-slot rule refusing the structural + * blank also composes — is what keeps the doors from drifting. * * ## What it refuses, and what that was doing before * diff --git a/packages/spec/src/automation/flow.zod.ts b/packages/spec/src/automation/flow.zod.ts index 12360e7a511..7785358b87b 100644 --- a/packages/spec/src/automation/flow.zod.ts +++ b/packages/spec/src/automation/flow.zod.ts @@ -23,6 +23,7 @@ import { retiredKey } from '../shared/retired-key'; import { retryPolicyShape } from '../shared/retry-policy.zod'; import { strictObject } from '../shared/strict-object'; import { collectFlowGraphs, parseFlowNodeRegions } from './control-flow.zod'; +import { predicateSlotRefusal, resolveFlowNodeExpressions } from './flow-node-expression-paths'; import { EndConfigSchema } from './builtin-node-config.zod'; import { APPROVAL_NODE_TYPE, APPROVAL_REVISE_NODE_TYPE } from './approval.zod'; export const FlowNodeAction = z.enum([ @@ -157,7 +158,10 @@ export const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[] = [ * `end` node has no executor and no descriptor, so the flow parse is the ONLY * door its config passes through — {@link parseEndNodeConfig} applies * {@link EndConfigSchema} to it. Every other type's `config` stays the - * executor's to close. + * executor's to close. One VALUE rule reaches into an open `config` at the + * flow level without closing its key set (#17493): a blank string in a slot + * the expression ledger declares with the `predicate` role is refused by the + * `FlowSchema` superRefine — see the block there for its scope. */ /** @@ -1288,6 +1292,52 @@ export const FlowSchema = lazySchema(() => strictObject( }); } + // A blank string in a ledger `predicate` slot (#17493, ruling A) — + // `decision`'s `config.conditions[].expression` and `screen`'s + // `config.fields[].visibleWhen`, the slots `FLOW_NODE_EXPRESSION_PATHS` + // declares with the `predicate` role. The flow parse is the first of the + // three doors that refuse it; `AutomationEngine.registerFlow` (which parses + // first) and `objectstack validate` are the other two, and all three answer + // with `predicateSlotRefusal` — the spec's one notion of an unauthorable + // predicate — so the doors cannot grow two sentences or two notions of + // "blank". A blank predicate is an author who meant to write a rule: the + // resolver used to skip it and `evaluateCondition` answered it `false`, so a + // `decision` branch carrying it was never taken, and the two sides agreeing + // was ruled no defence. + // + // Scoped on purpose, three ways: + // - STRINGS only. A non-string there (the `{ dialect, source }` envelope + // above all) is refused at the other two doors by the same function + // (#15572) and was never ruled at this one; refusing it here would narrow + // the flow parse's accept set past the ruling. + // - The ledger's `predicate` role only. A `flow-template` slot's blank is + // untouched (no validator implements that dialect), and so is the + // structural `config.condition`, which the ledger does not list — its + // blank is refused at `registerFlow` and `objectstack validate` (#17322, + // #17495), and a node's open `config` still carries no parse door for it. + // - A VALUE rule, never a key-set closure: the node `config` stays the open + // record the header of this module describes. + // + // Walked with `collectFlowGraphs`, like the two refusals above, so a + // `decision` inside an ADR-0031 region body is refused here too, anchored at + // the path the author wrote (`nodes.1.config.body.nodes.0.config…`). + for (const graph of collectFlowGraphs(flow)) { + graph.nodes.forEach((node, index) => { + const type: unknown = (node as { type?: unknown } | null)?.type; + if (typeof type !== 'string') return; + for (const found of resolveFlowNodeExpressions(type, (node as { config?: unknown }).config)) { + if (found.entry.role !== 'predicate' || typeof found.value !== 'string') continue; + const refusal = predicateSlotRefusal(found.value); + if (!refusal) continue; + ctx.addIssue({ + code: 'custom', + path: [...graph.path, 'nodes', index, 'config', ...ledgerPathSegments(found.path)], + message: refusal.message, + }); + } + }); + } + // Edges (#14964): every reader of `edges[].id` assumes the ids are unique — // a designer, a BPMN export, a flow diff, any traversal that dedupes by id — // while nothing enforced it: two edges carrying one id parsed, shipped @@ -1313,6 +1363,25 @@ export const FlowSchema = lazySchema(() => strictObject( }); })); +/** + * A ledger path as the resolver fills it in (`conditions[0].expression`, + * `fields[2].visibleWhen`) → the Zod issue path segments it names + * (`['conditions', 0, 'expression']`), so a flow-parse refusal of a ledger + * slot (#17493) is anchored where the author wrote the value, the address + * `formatZodError` prints for any other node key. A hoisted `function` for the + * same reason {@link flowNodeObject} is one. + */ +function ledgerPathSegments(path: string): (string | number)[] { + const segments: (string | number)[] = []; + for (const part of path.split('.')) { + const bracket = part.indexOf('['); + segments.push(bracket === -1 ? part : part.slice(0, bracket)); + if (bracket === -1) continue; + for (const index of part.slice(bracket).matchAll(/\[(\d+)\]/g)) segments.push(Number(index[1])); + } + return segments; +} + /** * Type-safe factory for creating flow definitions. * From 065ea98a382f26d48959558353c3809b902072a8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:18:12 +0000 Subject: [PATCH 2/7] wip: pins at the three doors, re-judged #15572 pins, ADR-0087 entry Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- .../lint/src/validate-expressions.test.ts | 104 ++++++++++++- packages/lint/src/validate-expressions.ts | 5 + .../builtin/config-expression-ledger.test.ts | 18 ++- .../src/decision-predicate-envelope.test.ts | 27 +++- .../services/service-automation/src/engine.ts | 8 + .../src/predicate-slot-blank.test.ts | 134 ++++++++++++++++ .../flow-node-expression-paths.test.ts | 37 ++++- .../flow-predicate-slot-blank.test.ts | 145 ++++++++++++++++++ ...low-predicate-slot-blank-string-refused.ts | 78 ++++++++++ 9 files changed, 537 insertions(+), 19 deletions(-) create mode 100644 packages/services/service-automation/src/predicate-slot-blank.test.ts create mode 100644 packages/spec/src/automation/flow-predicate-slot-blank.test.ts create mode 100644 packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 484a2359336..4b6dd2dfa64 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -2235,10 +2235,18 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(atSlot(['a > 1'])[0].message).toContain('Found an array'); }); - it('leaves string predicates alone — including the whitespace-only one', () => { - // The card states this boundary explicitly so nobody "fixes" it: a - // whitespace-only STRING is "not authored" on both sides and stays so. - expect(atSlot(' ')).toHaveLength(0); + it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => { + // RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. This + // pinned `atSlot(' ')` at ZERO findings: "The card states this + // boundary explicitly so nobody 'fixes' it: a whitespace-only STRING + // is 'not authored' on both sides and stays so." Both sides do still + // agree; the ruling is that the agreement is no defence when the + // author's rule is silently dropped, so the blank is refused here — on + // the same rule and sentence as the envelope above. + const blank = atSlot(' '); + expect(blank).toHaveLength(1); + expect(blank[0].severity).toBe('error'); + expect(blank[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); expect(atSlot("lead_record.status == 'converted'")).toHaveLength(0); }); }); @@ -4293,3 +4301,91 @@ describe('masterDetailCount — an unreadable `reference` carrier is refused (#1 expect(parentScope[0]!.message).toMatch(/declares no `master_detail` relationships/); }); }); + +/** + * [#17493] (ruling A, 5651023407) — a blank string in a ledger `predicate` + * slot, at the THIRD door: `objectstack validate`'s expression pass. + * + * `decision`'s `config.conditions[].expression` and `screen`'s + * `config.fields[].visibleWhen` reported NOTHING for `''` / `' '`: the + * resolver skipped the blank as "not authored", so `validateStackExpressions` + * never saw it, and a branch carrying it was never taken at run time. The + * refusal is `predicateSlotRefusal`'s — the spec's one notion, shared with + * `FlowSchema.parse` and `registerFlow` — so the finding leads with the same + * published sentence the other two doors answer with. + * + * ⚠️ Through the CLI, `objectstack validate` meets these values first at its + * schema step (`FlowSchema.parse` refuses them there). This pass is what + * answers for a stack handed to `validateStackExpressions` directly, and it is + * what these pins drive. + */ +describe('a blank string in a ledger predicate slot (#17493)', () => { + const flowStack = (...middle: Record[]) => ({ + flows: [{ + name: 'blank_flow', + nodes: [{ id: 'start', type: 'start' }, ...middle], + edges: [], + }], + }); + const decision = (expression: unknown) => + ({ id: 'check', type: 'decision', config: { conditions: [{ label: 'Yes', expression }] } }); + const screen = (visibleWhen: unknown) => + ({ id: 'form', type: 'screen', config: { fields: [{ name: 'amount', type: 'number', visibleWhen }] } }); + const errorsOf = (stack: unknown) => + validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error'); + + describe.each(['', ' ', '\t\n '])('the blank %j', (blank) => { + it('decision branch `config.conditions[].expression` — one error, located at node and branch', () => { + const found = errorsOf(flowStack(decision(blank))); + expect(found).toHaveLength(1); + expect(found[0].severity).toBe('error'); + expect(found[0].where).toBe("flow 'blank_flow' · node 'check' (decision) decision branch expression at config.conditions[0].expression"); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(found[0].source).toBe(blank); + }); + + it('screen field `config.fields[].visibleWhen` — one error, located at node and field', () => { + const found = errorsOf(flowStack(screen(blank))); + expect(found).toHaveLength(1); + expect(found[0].severity).toBe('error'); + expect(found[0].where).toBe("flow 'blank_flow' · node 'form' (screen) screen field visibleWhen at config.fields[0].visibleWhen"); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + }); + + it('reaches a `decision` inside an ADR-0031 region body', () => { + const found = errorsOf(flowStack({ + id: 'sweep', type: 'loop', + config: { collection: '{items}', itemVariable: 'item', body: { nodes: [decision(' ')], edges: [] } }, + })); + expect(found).toHaveLength(1); + expect(found[0].where).toContain("loop 'sweep' body"); + expect(found[0].where).toContain('config.conditions[0].expression'); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + + describe('CONTROLS — what this must NOT move', () => { + it('RED CONTROL — the brace trap on the same slot still earns its own verdict, not the blank one', () => { + const found = errorsOf(flowStack(decision('{amount} > 1'))); + expect(found).toHaveLength(1); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(false); + }); + + it('a non-blank predicate and an absent one report nothing', () => { + expect(errorsOf(flowStack(decision('amount > 1')))).toHaveLength(0); + expect(errorsOf(flowStack(screen('amount > 0')))).toHaveLength(0); + expect(errorsOf(flowStack(screen(undefined)))).toHaveLength(0); + }); + + it('the structural `config.condition` keeps its own rule and sentence', () => { + const found = errorsOf(flowStack({ id: 'gate', type: 'decision', config: { condition: ' ' } })); + expect(found).toHaveLength(1); + expect(found[0].message).toContain(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(false); + }); + + it("a `flow-template` slot's blank is untouched", () => { + expect(errorsOf(flowStack({ id: 'sweep', type: 'loop', config: { collection: ' ' } }))).toHaveLength(0); + }); + }); +}); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 7c085e15ace..10ece409e54 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -1167,6 +1167,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { // anything tries to read a source out of it. The refusal is the spec's, // shared with the engine's `registerFlow` pass: `error`, because that pass // throws, and a shape build refuses must not pass author time. + // + // [#17493] The same call refuses a BLANK string too (the resolver emits it + // for this role). `objectstack validate` meets that value first at its + // schema step — `FlowSchema.parse` refuses it — so this is the pass that + // answers for a stack handed to `validateStackExpressions` directly. const shapeRefusal = predicateSlotRefusal(raw); if (shapeRefusal) { issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' }); diff --git a/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts b/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts index 72625d2214c..b8d8d7628dc 100644 --- a/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts +++ b/packages/services/service-automation/src/builtin/config-expression-ledger.test.ts @@ -281,14 +281,28 @@ describe('resolveFlowNodeExpressions — path resolution (#4027)', () => { expect(found[0].entry.role).toBe('flow-template'); }); - it('skips absent and empty values rather than inventing findings', () => { + it('skips absent values rather than inventing findings', () => { expect(resolveFlowNodeExpressions('screen', {})).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]); - expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] })).toEqual([]); // A repeater authored as a non-array must not throw. expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]); }); + // RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. The test + // above used to pin a whitespace-only `visibleWhen` as skipped too — "an + // empty value, not a finding" — on #15572's ground that the resolver and the + // evaluator treated the blank the same way. That agreement was ruled no + // defence: a blank predicate is an author's rule that was never written. So + // the blank is EMITTED for a `predicate` slot, for `registerFlow` and + // `objectstack validate` to refuse, and still skipped for a `flow-template` + // one, which the ruling did not reach. + it('emits a blank string in a predicate slot for the consumer to refuse (#17493)', () => { + expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] }) + .map((f) => [f.path, f.value, f.entry.role])) + .toEqual([['fields[0].visibleWhen', ' ', 'predicate']]); + expect(resolveFlowNodeExpressions('loop', { collection: ' ' })).toEqual([]); + }); + // [#15572] A NON-string in a predicate slot is no longer skipped. It was // skipped as "a type violation for the schema pass to report", and for the // schemaless node types — `decision` publishes no descriptor `configSchema`, diff --git a/packages/services/service-automation/src/decision-predicate-envelope.test.ts b/packages/services/service-automation/src/decision-predicate-envelope.test.ts index b08c49c3f6f..100e0268012 100644 --- a/packages/services/service-automation/src/decision-predicate-envelope.test.ts +++ b/packages/services/service-automation/src/decision-predicate-envelope.test.ts @@ -102,14 +102,29 @@ describe('decision branch predicate — envelope in a `z.string()` slot (#15572) }); /** - * The boundary the card states explicitly, so nobody "fixes" it: a - * whitespace-only STRING predicate behaves consistently on both sides — - * "not authored" at the ledger, `false` at the evaluator — and is left - * exactly as it was. Only the envelope shape moved. + * RE-JUDGED IN PLACE (#17493, ruling A 5651023407) — not deleted, because + * the boundary it drew was a real decision and the reason it moved belongs + * next to it. + * + * What it pinned: "The boundary the card states explicitly, so nobody + * 'fixes' it: a whitespace-only STRING predicate behaves consistently on + * both sides — 'not authored' at the ledger, `false` at the evaluator — and + * is left exactly as it was. Only the envelope shape moved." So + * `decisionFlow('str_ws', ' ')` was asserted to register. + * + * Why it moved: the ground is still TRUE — the two sides do agree — and + * was ruled insufficient. A blank branch predicate is an author who meant + * to write a rule; the parser skipping it and the evaluator answering + * `false` only proves the platform did not crash, while the branch is never + * taken and nothing says so. The ruling is the third instance of one rule + * (#17322's `config.condition`, #15811's evaluated `source`), so the blank + * string is now refused here too — by `FlowSchema.parse` inside + * `registerFlow`, under this slot's own sentence. A non-blank string and an + * absent predicate still register, exactly as this test always said. */ - it('leaves string predicates alone — including the whitespace-only one', () => { + it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => { expect(() => engine.registerFlow('str_ok', decisionFlow('str_ok', 'record.rating >= 4'))).not.toThrow(); - expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', ' '))).not.toThrow(); + expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', ' '))).toThrow(PREDICATE_SLOT_STRING_REFUSAL); expect(() => engine.registerFlow('str_absent', decisionFlow('str_absent', undefined))).not.toThrow(); }); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index c29d77c7b88..c9d47ef9737 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -9522,6 +9522,14 @@ export class AutomationEngine implements IAutomationService { // evaluation must be one set, and the refusal itself is the // spec's, shared with `objectstack validate` so build and // author time cannot disagree about the shape. + // + // [#17493] The same call now refuses a BLANK string too: the + // resolver emits it for this role, and `predicateSlotRefusal` + // refuses it under the same sentence. ⚠️ For that value this + // is the second line, not the first — `FlowSchema.parse` + // (in `canonicalizeStoredFlow`, above) refuses it before + // this pass runs, the same way it meets a blank + // `edge.condition` before `checkStructuralCondition` does. const shapeRefusal = predicateSlotRefusal(found.value); if (shapeRefusal) { failures.push(` • ${slotWhere}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``); diff --git a/packages/services/service-automation/src/predicate-slot-blank.test.ts b/packages/services/service-automation/src/predicate-slot-blank.test.ts new file mode 100644 index 00000000000..fe03e040902 --- /dev/null +++ b/packages/services/service-automation/src/predicate-slot-blank.test.ts @@ -0,0 +1,134 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #17493 (ruling A, 5651023407) — a blank string in a ledger `predicate` slot + * is refused at `AutomationEngine.registerFlow`, the second of the three doors. + * + * The two slots are `decision`'s `config.conditions[].expression` and + * `screen`'s `config.fields[].visibleWhen`. Until this card both registered a + * `''` / `' '` clean: the resolver skipped the blank as "not authored", and a + * `decision` branch carrying it went to `evaluateCondition`, answered `false`, + * and was never taken — with nothing said at any layer. #15572 had pinned that + * admission as correct because the two sides agreed; the ruling is that + * agreement is no defence (`decision-predicate-envelope.test.ts` carries that + * pin, re-judged in place). + * + * ## Which gate answers, measured rather than assumed + * + * `registerFlow` parses first (`canonicalizeStoredFlow` → `FlowSchema.parse`), + * and the flow parse refuses these values itself since this card — so the + * refusal this door hands back is the PARSE's: a Zod issue with code `custom`, + * anchored at the slot. That is what these pins assert, because it is what a + * caller receives (the `/automation` write doors map exactly that issue to a + * `VALIDATION_ERROR` field). The engine's own ledger pass refuses the same + * value through the same `predicateSlotRefusal`, one step later — the same + * two-layer shape a blank `edge.condition` has had since #15807. + */ +import { describe, expect, it, vi } from 'vitest'; +import { PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation'; +import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec'; + +import { AutomationEngine } from './engine.js'; + +const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any; + +const BLANKS = ['', ' ', '\t\n '] as const; + +type Node = Record; + +function flowWith(...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle, { id: 'end', type: 'end', label: 'End' }]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name: 'blank_flow', label: 'Blank flow', type: 'autolaunched', nodes, edges }; +} + +const decision = (...expressions: unknown[]): Node => ({ + id: 'branch', type: 'decision', label: 'Branch', + config: { conditions: expressions.map((expression, i) => ({ label: `b${i}`, expression })) }, +}); + +const screen = (visibleWhen: unknown): Node => ({ + id: 'form', type: 'screen', label: 'Form', + config: { fields: [{ name: 'amount', label: 'Amount', type: 'number', visibleWhen }] }, +}); + +const loopAround = (inner: Node): Node => ({ + id: 'sweep', type: 'loop', label: 'Sweep', + config: { collection: '{items}', itemVariable: 'item', body: { nodes: [inner], edges: [] } }, +}); + +/** What `registerFlow` threw, or `undefined` when it registered. */ +function refusalOf(engine: AutomationEngine, flow: unknown): { message: string; issues?: Array<{ code: string; path: unknown[]; message: string }> } | undefined { + try { + engine.registerFlow('blank_flow', flow as never); + return undefined; + } catch (e) { + return e as never; + } +} + +describe('registerFlow refuses a blank string in a ledger predicate slot (#17493)', () => { + describe.each(BLANKS)('the blank %j', (blank) => { + it('decision branch `config.conditions[].expression` — refused, code `custom`, at the branch; nothing registered', async () => { + const engine = new AutomationEngine(silentLogger); + const refusal = refusalOf(engine, flowWith(decision(blank))); + expect(refusal).toBeDefined(); + expect(refusal!.issues?.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'conditions', 0, 'expression']], + ]); + expect(refusal!.issues![0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(await engine.getFlow('blank_flow')).toBeNull(); + }); + + it('screen field `config.fields[].visibleWhen` — refused, code `custom`, at the field; nothing registered', async () => { + const engine = new AutomationEngine(silentLogger); + const refusal = refusalOf(engine, flowWith(screen(blank))); + expect(refusal).toBeDefined(); + expect(refusal!.issues?.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'fields', 0, 'visibleWhen']], + ]); + expect(refusal!.issues![0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + expect(await engine.getFlow('blank_flow')).toBeNull(); + }); + }); + + it('refuses a blank branch inside an ADR-0031 region body, anchored where the author wrote it', () => { + const refusal = refusalOf(new AutomationEngine(silentLogger), flowWith(loopAround(decision(' ')))); + expect(refusal?.issues?.map((i) => [i.code, i.path])).toEqual([ + ['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'conditions', 0, 'expression']], + ]); + }); + + describe('CONTROLS — what this must NOT move', () => { + it('a non-blank predicate still registers, on both slots', () => { + expect(refusalOf(new AutomationEngine(silentLogger), flowWith(decision('record.amount > 10', 'true')))).toBeUndefined(); + expect(refusalOf(new AutomationEngine(silentLogger), flowWith(screen('amount > 0')))).toBeUndefined(); + }); + + it('an absent predicate still registers', () => { + expect(refusalOf(new AutomationEngine(silentLogger), flowWith(screen(undefined)))).toBeUndefined(); + }); + + it('the structural `config.condition` keeps its own rule and sentence', () => { + const refusal = refusalOf(new AutomationEngine(silentLogger), + flowWith({ id: 'gate', type: 'decision', label: 'G', config: { condition: ' ' } })); + expect(refusal?.message).toContain(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + expect(refusal?.message).not.toContain(PREDICATE_SLOT_STRING_REFUSAL); + }); + + it('the EDGE slot keeps its own rule and sentence', () => { + const flow = flowWith(decision('true')); + (flow.edges[1] as Record).condition = ' '; + const refusal = refusalOf(new AutomationEngine(silentLogger), flow); + expect(refusal?.message).toContain(EVALUATED_EXPRESSION_SOURCE_REQUIRED); + expect(refusal?.message).not.toContain(PREDICATE_SLOT_STRING_REFUSAL); + }); + + it('the EVALUATOR is untouched: a blank predicate a stored row still carries answers `false`', () => { + // `registerFlow` refuses the value; `evaluateCondition` is the shared + // evaluator and a public method, so its answer is its own contract. + const engine = new AutomationEngine(silentLogger); + expect(engine.evaluateCondition({ dialect: 'cel', source: ' ' } as never, new Map())).toBe(false); + }); + }); +}); 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 41aef5aba1d..06fc02c6c82 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.test.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.test.ts @@ -182,13 +182,24 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu expect(found.every((f) => f.entry.role === 'predicate')).toBe(true); }); - it('absent and empty values in a string-role slot are skipped', () => { + it('absent values in a string-role slot are skipped; a blank one is skipped only where no refusal applies', () => { expect(resolveFlowNodeExpressions('screen', {})).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]); - // A whitespace-only STRING is "not authored", on this side and at the - // evaluator alike — consistent on both sides, and deliberately left alone - // (#15572 changed the non-string rule, never the string one). - expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] })).toEqual([]); + // RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. This line + // pinned `[]`: a whitespace-only STRING was "not authored", on this side + // and at the evaluator alike, and #15572 left the string rule alone on + // that ground — consistent on both sides. The ground is still true and was + // ruled no defence: a blank predicate is an author's rule that was never + // written, so the resolver now EMITS it for the `predicate` role and every + // door refuses it through `predicateSlotRefusal`. + expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] }) + .map((f) => [f.path, f.value, f.entry.role])).toEqual([['fields[0].visibleWhen', ' ', 'predicate']]); + expect(resolveFlowNodeExpressions('decision', { conditions: [{ label: 'x', expression: '' }] }) + .map((f) => [f.path, f.value])).toEqual([['conditions[0].expression', '']]); + // …and ONLY for that role: a `flow-template` slot's blank is still skipped, + // because no validator implements that dialect and the ruling did not + // reach it. + expect(resolveFlowNodeExpressions('loop', { collection: ' ' })).toEqual([]); expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]); // A `flow-template` slot keeps the old rule: no validator implements that // dialect, so emitting a non-string there would hand every consumer a @@ -219,12 +230,24 @@ describe('every pre-#14149 entry resolves byte-identically (the ratchet\'s fixtu }); describe('predicateSlotRefusal (#15572)', () => { - it('says nothing about a string — what it SAYS is validateExpression\'s business', () => { + it('says nothing about a non-blank string — what it SAYS is validateExpression\'s business', () => { expect(predicateSlotRefusal('record.rating >= 4')).toBeUndefined(); // Including a string that is itself malformed: the shape is right, so // this function is done and the CEL parse issues the verdict. expect(predicateSlotRefusal('{record.rating} >= 4')).toBeUndefined(); - expect(predicateSlotRefusal('')).toBeUndefined(); + }); + + it('REFUSES a string that is blank after trimming, under the same sentence (#17493)', () => { + // RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted: the + // test above used to end `expect(predicateSlotRefusal('')).toBeUndefined()` + // on #15572's ground that the blank was treated the same on both sides. + // That ground was ruled insufficient, so the blank moved here. + for (const blank of ['', ' ', '\t\n ']) { + const refusal = predicateSlotRefusal(blank); + expect(refusal?.message.startsWith(PREDICATE_SLOT_STRING_REFUSAL), JSON.stringify(blank)).toBe(true); + // Attributed to what the author wrote, whitespace and all. + expect(refusal?.source).toBe(blank); + } }); it('refuses an envelope and attributes it to the envelope\'s own source', () => { diff --git a/packages/spec/src/automation/flow-predicate-slot-blank.test.ts b/packages/spec/src/automation/flow-predicate-slot-blank.test.ts new file mode 100644 index 00000000000..b6ddcf7f4ee --- /dev/null +++ b/packages/spec/src/automation/flow-predicate-slot-blank.test.ts @@ -0,0 +1,145 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #17493 (ruling A, 5651023407) — a blank string in a ledger `predicate` slot + * is refused at `FlowSchema.parse`, the first of the three doors. + * + * The two slots the expression ledger declares with the `predicate` role — + * `decision`'s `config.conditions[].expression` and `screen`'s + * `config.fields[].visibleWhen` — accepted `''` / `' '` at every door until + * this card: the resolver skipped the blank as "not authored" and the + * evaluator answered it `false`, so a `decision` branch carrying it was never + * taken and nothing said so. That the two sides agreed was ruled no defence. + * + * What this file pins is the flow-parse door's record of the refusal — the + * issue `code`, the `path` it is anchored at, and the published sentence it + * leads with (asserted off the spec's own export, never re-spelled) — for each + * slot, at the top level and inside an ADR-0031 region body. The other two + * doors are pinned in their own packages (`service-automation`'s + * `predicate-slot-blank.test.ts`, `lint`'s `validate-expressions.test.ts`). + * + * The CONTROLS are the other half: the rule reaches exactly what was ruled — + * strings, in `predicate` slots — and moves no neighbouring accept set. + */ + +import { describe, expect, it } from 'vitest'; + +import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '../shared/expression.zod'; +import { PREDICATE_SLOT_STRING_REFUSAL } from './flow-node-expression-paths'; +import { FlowSchema } from './flow.zod'; + +const BLANKS = ['', ' ', '\t\n '] as const; + +type Node = Record; + +/** start → → end, chained by unconditional edges. */ +function flowWith(...middle: Node[]) { + const nodes: Node[] = [{ id: 'start', type: 'start', label: 'Start' }, ...middle, { id: 'end', type: 'end', label: 'End' }]; + const edges = nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: String(nodes[i].id), target: String(n.id) })); + return { name: 'blank_probe', label: 'Blank probe', type: 'autolaunched', nodes, edges }; +} + +const decision = (...expressions: unknown[]): Node => ({ + id: 'branch', + type: 'decision', + label: 'Branch', + config: { conditions: expressions.map((expression, i) => ({ label: `b${i}`, expression })) }, +}); + +const screen = (visibleWhen: unknown): Node => ({ + id: 'form', + type: 'screen', + label: 'Form', + config: { fields: [{ name: 'amount', label: 'Amount', type: 'number', visibleWhen }] }, +}); + +const loopAround = (inner: Node): Node => ({ + id: 'sweep', + type: 'loop', + label: 'Sweep', + config: { collection: '{items}', itemVariable: 'item', body: { nodes: [inner], edges: [] } }, +}); + +function issuesOf(flow: unknown) { + const result = FlowSchema.safeParse(flow); + return result.success ? [] : result.error.issues; +} + +const predicateIssues = (flow: unknown) => + issuesOf(flow).filter((i) => String(i.message).startsWith(PREDICATE_SLOT_STRING_REFUSAL)); + +describe('FlowSchema.parse refuses a blank string in a ledger predicate slot (#17493)', () => { + describe.each(BLANKS)('the blank %j', (blank) => { + it('decision branch `config.conditions[].expression` — code `custom`, anchored at the branch', () => { + const issues = issuesOf(flowWith(decision(blank))); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('custom'); + expect(issues[0].path).toEqual(['nodes', 1, 'config', 'conditions', 0, 'expression']); + expect(issues[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + + it('screen field `config.fields[].visibleWhen` — code `custom`, anchored at the field', () => { + const issues = issuesOf(flowWith(screen(blank))); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('custom'); + expect(issues[0].path).toEqual(['nodes', 1, 'config', 'fields', 0, 'visibleWhen']); + expect(issues[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + }); + + it('names WHICH branch: a blank second branch is anchored at index 1, and the valid first one is not', () => { + const issues = issuesOf(flowWith(decision('record.amount > 10', ' '))); + expect(issues.map((i) => i.path)).toEqual([['nodes', 1, 'config', 'conditions', 1, 'expression']]); + }); + + it('reaches a `decision` inside an ADR-0031 region body, anchored where the author wrote it', () => { + const issues = issuesOf(flowWith(loopAround(decision(' ')))); + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('custom'); + expect(issues[0].path).toEqual(['nodes', 1, 'config', 'body', 'nodes', 0, 'config', 'conditions', 0, 'expression']); + expect(issues[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true); + }); + + describe('CONTROLS — what this rule must NOT reach', () => { + it('a non-blank predicate parses unchanged, on both slots and in a region body', () => { + expect(FlowSchema.safeParse(flowWith(decision('record.amount > 10', 'true'))).success).toBe(true); + expect(FlowSchema.safeParse(flowWith(screen('amount > 0'))).success).toBe(true); + expect(FlowSchema.safeParse(flowWith(loopAround(decision('item.done')))).success).toBe(true); + }); + + it('an absent predicate is still not a malformed one', () => { + expect(FlowSchema.safeParse(flowWith(screen(undefined))).success).toBe(true); + expect(FlowSchema.safeParse(flowWith({ id: 'branch', type: 'decision', label: 'B', config: {} })).success).toBe(true); + }); + + it('a NON-string in a predicate slot is not this door\'s to refuse — #15572 refuses it at the other two', () => { + // Scoped to what was ruled: the flow parse's accept set moves for blank + // STRINGS only. The envelope is refused at `registerFlow` and + // `objectstack validate` by the same `predicateSlotRefusal`, unchanged. + expect(predicateIssues(flowWith(decision({ dialect: 'cel', source: 'x > 1' })))).toEqual([]); + expect(predicateIssues(flowWith(screen({ dialect: 'cel', source: ' ' })))).toEqual([]); + }); + + it('the EDGE slot keeps its own rule and sentence — unchanged', () => { + const flow = flowWith(decision('true')); + (flow.edges[1] as Record).condition = ' '; + const issues = issuesOf(flow); + expect(issues.length).toBeGreaterThan(0); + expect(issues.every((i) => i.path[0] === 'edges')).toBe(true); + expect(issues.some((i) => String(i.message).includes(EVALUATED_EXPRESSION_SOURCE_REQUIRED))).toBe(true); + expect(predicateIssues(flow)).toEqual([]); + }); + + it('the STRUCTURAL `config.condition` is not a ledger slot — this rule does not reach it', () => { + // Its blank is refused at `registerFlow` and `objectstack validate` + // (#17322, #17495) by the evaluated-slot rule, not here. + expect(predicateIssues(flowWith({ id: 'gate', type: 'decision', label: 'G', config: { condition: ' ' } }))).toEqual([]); + }); + + it('a `flow-template` slot\'s blank is untouched — no validator implements that dialect', () => { + const loop = loopAround({ id: 'noop', type: 'assignment', label: 'N', config: { assignments: { x: '1' } } }); + (loop.config as Record).collection = ' '; + expect(predicateIssues(flowWith(loop))).toEqual([]); + }); + }); +}); diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts new file mode 100644 index 00000000000..746e47ae814 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts @@ -0,0 +1,78 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// The ledger `predicate` slots' half of the blank-predicate rule. A SEPARATE +// entry from `flow-edge-condition-evaluated-slot-source-required` on purpose: +// that one carries the structural slots (`edges[].condition`, +// `config.condition`), refused by the evaluated-slot rule under +// EVALUATED_EXPRESSION_SOURCE_REQUIRED, where removing a blank condition +// INVERTS the edge. These slots are declared `z.string()`, are refused under +// PREDICATE_SLOT_STRING_REFUSAL, and removing the blank is behaviour- +// preserving — a different prescription, which one entry cannot carry for both. +// +// 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-predicate-slot-blank-string-refused', + surface: + 'the two ledger predicate slots on a flow node — config.conditions[].expression on a decision ' + + 'node (a branch predicate) and config.fields[].visibleWhen on a screen node (a field visibility ' + + 'predicate) — authored as a string that is blank after trimming (\'\', \' \', a tab or a ' + + 'newline), at any depth including an ADR-0031 region body. Reachable wherever a flow is ' + + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' + + 'objectstack validate, a POST /flows body, and a flow row already sitting in sys_metadata', + replacement: + 'the predicate the branch or field was meant to test, as non-blank bare CEL text ' + + '(`expression: \'record.amount > 10\'`, `visibleWhen: \'amount > 0\'`); or REMOVE the blank. ' + + 'On a screen field, drop the `visibleWhen` key: an absent `visibleWhen` shows the field ' + + 'unconditionally, which is what a blank one already did at run time (the resume contract ' + + 'treated it as absent, and the renderer fell back to showing the field). On a decision ' + + 'branch, drop the whole `conditions[i]` element: `expression` is required by ' + + '`DecisionConditionSchema`, so a branch cannot keep its label without it, and a branch whose ' + + 'predicate was blank was never taken, so dropping it changes no run. ⚠️ Removal is ' + + 'behaviour-preserving HERE, unlike on a structural condition, where dropping a blank ' + + '`condition` turns a never-firing edge into an always-firing one ' + + '(`flow-edge-condition-evaluated-slot-source-required`)', + reason: + 'Card #17493, ruling A (5651023407). Both slots are declared bare CEL text (`z.string()`) ' + + 'and both admitted a blank string at every door: the expression ledger resolver skipped it ' + + 'as "not authored", and `AutomationEngine.evaluateCondition` answered it `false` — so a ' + + 'decision branch carrying it was never taken, with nothing said at any layer, and a screen ' + + 'field carrying it was shown with its predicate ignored. #15572 had pinned that ' + + 'admission as correct because the two sides agreed. The ruling is that self-consistency ' + + 'between parser and evaluator is not a defence when the author\'s intent is silently ' + + 'dropped — the third instance of one rule, after #17322 (the structural `config.condition`) ' + + 'and #15811 (a blank evaluated `source`). The blank is now refused at `FlowSchema.parse`, ' + + 'at `AutomationEngine.registerFlow` (which parses first) and at `objectstack validate`, all ' + + 'three through `predicateSlotRefusal`, leading with `PREDICATE_SLOT_STRING_REFUSAL`. ' + + '⚠️ No D2 conversion, and the reason is the judgment this entry delegates: the blank is ' + + 'where an author meant to write a rule, and the platform cannot tell a predicate somebody ' + + 'forgot from one they meant to delete. Removing it preserves what ran; writing it is what ' + + 'the author intended; only the author knows which. ' + + '⚠️ A flow ALREADY STORED with such a blank no longer registers at all, not just that ' + + 'branch or field: `registerFlow` parses through `canonicalizeStoredFlow` → ' + + '`FlowSchema.parse`, and each boot path in `service-automation/src/plugin.ts` logs one ' + + '`warn` naming the flow and continues — its trigger is never armed. ' + + 'ADR-0087, ADR-0032.', + acceptanceCriteria: + 'Grep every flow node in `defineStack({ flows })` sources, exported stacks, `POST /flows` ' + + 'bodies and every flow row in `sys_metadata` — including nodes inside a `loop` / ' + + '`parallel` / `try_catch` region body — for a `decision` node whose ' + + '`config.conditions[i].expression`, or a `screen` node whose `config.fields[i].visibleWhen`, ' + + 'is a string that is empty after trimming. Each refusal names the node and the branch or ' + + 'field, which is the TODO\'s locator: `FlowSchema.parse` anchors a `custom` issue at ' + + '`nodes.N.config.conditions.I.expression` (or `…config.fields.I.visibleWhen`, or the region ' + + 'path `nodes.N.config.body.nodes.M.config…`), and `objectstack validate` prints the same ' + + 'path; `validateStackExpressions` phrases it as ' + + '`node \'check\' (decision) decision branch expression at config.conditions[0].expression`. ' + + 'For each hit decide, per the `replacement` note, whether to write the predicate or to ' + + 'remove it — and on a decision branch removing means the whole branch. Two proofs. (1) For ' + + 'a stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and ' + + 'confirm each flow REGISTERS: no `failed to register flow` warn for it (the three boot ' + + 'paths spell it `[Automation] failed to register flow`, `[Automation] flow re-sync: failed ' + + 'to register flow` and `[Automation] cold-boot flow bind: failed to register flow`) — that ' + + 'warn line is the locator for a row that exists only in `sys_metadata`. A non-blank ' + + 'predicate parses and registers byte-identically to before, and a non-string in these ' + + 'slots keeps its own earlier refusal (at `registerFlow` and `objectstack validate`).', +}; From 6445eab9dfd44f1dfe2a18f8fc33d7c0328710d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:18:29 +0000 Subject: [PATCH 3/7] wip(spec): regenerate the migration registry Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- packages/spec/src/migrations/registry.ts | 74 ++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index dc578333c57..e0a9f77a8df 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -9058,6 +9058,80 @@ const step18: MigrationStep = { + "slot phrase. A flow that boots without that warn is unaffected; every structural " + 'condition carrying a non-blank `source` parses byte-identically to before.', }, + // The ledger `predicate` slots' half of the blank-predicate rule. A SEPARATE + // entry from `flow-edge-condition-evaluated-slot-source-required` on purpose: + // that one carries the structural slots (`edges[].condition`, + // `config.condition`), refused by the evaluated-slot rule under + // EVALUATED_EXPRESSION_SOURCE_REQUIRED, where removing a blank condition + // INVERTS the edge. These slots are declared `z.string()`, are refused under + // PREDICATE_SLOT_STRING_REFUSAL, and removing the blank is behaviour- + // preserving — a different prescription, which one entry cannot carry for both. + // + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code + // span already, and a nested backtick would close it. + { + id: 'flow-predicate-slot-blank-string-refused', + surface: + 'the two ledger predicate slots on a flow node — config.conditions[].expression on a decision ' + + 'node (a branch predicate) and config.fields[].visibleWhen on a screen node (a field visibility ' + + 'predicate) — authored as a string that is blank after trimming (\'\', \' \', a tab or a ' + + 'newline), at any depth including an ADR-0031 region body. Reachable wherever a flow is ' + + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' + + 'objectstack validate, a POST /flows body, and a flow row already sitting in sys_metadata', + replacement: + 'the predicate the branch or field was meant to test, as non-blank bare CEL text ' + + '(`expression: \'record.amount > 10\'`, `visibleWhen: \'amount > 0\'`); or REMOVE the blank. ' + + 'On a screen field, drop the `visibleWhen` key: an absent `visibleWhen` shows the field ' + + 'unconditionally, which is what a blank one already did at run time (the resume contract ' + + 'treated it as absent, and the renderer fell back to showing the field). On a decision ' + + 'branch, drop the whole `conditions[i]` element: `expression` is required by ' + + '`DecisionConditionSchema`, so a branch cannot keep its label without it, and a branch whose ' + + 'predicate was blank was never taken, so dropping it changes no run. ⚠️ Removal is ' + + 'behaviour-preserving HERE, unlike on a structural condition, where dropping a blank ' + + '`condition` turns a never-firing edge into an always-firing one ' + + '(`flow-edge-condition-evaluated-slot-source-required`)', + reason: + 'Card #17493, ruling A (5651023407). Both slots are declared bare CEL text (`z.string()`) ' + + 'and both admitted a blank string at every door: the expression ledger resolver skipped it ' + + 'as "not authored", and `AutomationEngine.evaluateCondition` answered it `false` — so a ' + + 'decision branch carrying it was never taken, with nothing said at any layer, and a screen ' + + 'field carrying it was shown with its predicate ignored. #15572 had pinned that ' + + 'admission as correct because the two sides agreed. The ruling is that self-consistency ' + + 'between parser and evaluator is not a defence when the author\'s intent is silently ' + + 'dropped — the third instance of one rule, after #17322 (the structural `config.condition`) ' + + 'and #15811 (a blank evaluated `source`). The blank is now refused at `FlowSchema.parse`, ' + + 'at `AutomationEngine.registerFlow` (which parses first) and at `objectstack validate`, all ' + + 'three through `predicateSlotRefusal`, leading with `PREDICATE_SLOT_STRING_REFUSAL`. ' + + '⚠️ No D2 conversion, and the reason is the judgment this entry delegates: the blank is ' + + 'where an author meant to write a rule, and the platform cannot tell a predicate somebody ' + + 'forgot from one they meant to delete. Removing it preserves what ran; writing it is what ' + + 'the author intended; only the author knows which. ' + + '⚠️ A flow ALREADY STORED with such a blank no longer registers at all, not just that ' + + 'branch or field: `registerFlow` parses through `canonicalizeStoredFlow` → ' + + '`FlowSchema.parse`, and each boot path in `service-automation/src/plugin.ts` logs one ' + + '`warn` naming the flow and continues — its trigger is never armed. ' + + 'ADR-0087, ADR-0032.', + acceptanceCriteria: + 'Grep every flow node in `defineStack({ flows })` sources, exported stacks, `POST /flows` ' + + 'bodies and every flow row in `sys_metadata` — including nodes inside a `loop` / ' + + '`parallel` / `try_catch` region body — for a `decision` node whose ' + + '`config.conditions[i].expression`, or a `screen` node whose `config.fields[i].visibleWhen`, ' + + 'is a string that is empty after trimming. Each refusal names the node and the branch or ' + + 'field, which is the TODO\'s locator: `FlowSchema.parse` anchors a `custom` issue at ' + + '`nodes.N.config.conditions.I.expression` (or `…config.fields.I.visibleWhen`, or the region ' + + 'path `nodes.N.config.body.nodes.M.config…`), and `objectstack validate` prints the same ' + + 'path; `validateStackExpressions` phrases it as ' + + '`node \'check\' (decision) decision branch expression at config.conditions[0].expression`. ' + + 'For each hit decide, per the `replacement` note, whether to write the predicate or to ' + + 'remove it — and on a decision branch removing means the whole branch. Two proofs. (1) For ' + + 'a stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and ' + + 'confirm each flow REGISTERS: no `failed to register flow` warn for it (the three boot ' + + 'paths spell it `[Automation] failed to register flow`, `[Automation] flow re-sync: failed ' + + 'to register flow` and `[Automation] cold-boot flow bind: failed to register flow`) — that ' + + 'warn line is the locator for a row that exists only in `sys_metadata`. A non-blank ' + + 'predicate parses and registers byte-identically to before, and a non-string in these ' + + 'slots keeps its own earlier refusal (at `registerFlow` and `objectstack validate`).', + }, { id: 'hook-register-undispatched-lifecycle-event-refused', surface: From dd251cb112f71d2d41d0f075453463fd2bf99f86 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 08:35:51 +0000 Subject: [PATCH 4/7] =?UTF-8?q?wip:=20changeset=20(minor=20+=20BREAKING,?= =?UTF-8?q?=20Clause-=E2=91=A1=20no=20(narrowing),=20adr-0087=20registered?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- .../17493-predicate-slot-blank-refused.md | 54 +++++++++++++++++++ .../src/predicate-slot-blank.test.ts | 6 ++- 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 .changeset/17493-predicate-slot-blank-refused.md diff --git a/.changeset/17493-predicate-slot-blank-refused.md b/.changeset/17493-predicate-slot-blank-refused.md new file mode 100644 index 00000000000..7f756193566 --- /dev/null +++ b/.changeset/17493-predicate-slot-blank-refused.md @@ -0,0 +1,54 @@ +--- +'@objectstack/spec': minor +--- + +fix(spec)!: a blank string in a flow node's predicate slot — a `decision` branch `expression`, a screen field `visibleWhen` — is refused at authoring (#17493) + +Clause-②: no (narrowing) + + + +**BREAKING** — an accept-set narrowing on two authored flow-node slots, shipped as +`minor` under the launch-window convention (`check-changeset-no-major` refuses +`major` until GA; breaking-ness is carried by this banner and the ADR-0087 +disposition above, not by the level). + +**What changed.** A `decision` node's `config.conditions[].expression` and a +`screen` node's `config.fields[].visibleWhen` are declared bare CEL text. A string +that is blank after trimming (`''`, `' '`, a tab or a newline) used to be +accepted there by `FlowSchema.parse`, `AutomationEngine.registerFlow` and +`objectstack validate`, and was then read as "no predicate": the evaluator answers +a blank decision predicate `false`, so that branch was not taken, and nothing said +so. It is now refused at those doors — by `FlowSchema.parse` with a `custom` issue +anchored at the slot (for example `nodes.1.config.conditions.0.expression`), and +by `registerFlow` and `objectstack validate` through that same parse — with a +message that leads with the published `PREDICATE_SLOT_STRING_REFUSAL` sentence, +the one these slots already answered with for a non-string value. A flow stored +with such a value no longer registers: the boot log carries a +`failed to register flow` warn naming it, and its trigger is not armed. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `conditions: [{ label: 'high', expression: ' ' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` — or drop that branch | +| `fields: [{ name: 'reason', visibleWhen: '' }]` on a `screen` node | the predicate you meant — `visibleWhen: "status == 'rejected'"` — or drop the `visibleWhen` key | + +**One-line fix:** write the predicate, or remove it — drop `visibleWhen` to show +the field unconditionally, or drop the whole decision branch (a branch requires +its `expression`). + +Removing is behaviour-preserving on these two slots: a blank `visibleWhen` was +already read as absent at run time, and a branch with a blank predicate was not +taken. That is not true of a blank structural `condition` on an edge or a node, +where removing the key makes the step unconditional — see the separate +`flow-edge-condition-evaluated-slot-source-required` migration entry. + +**Unchanged.** A non-blank predicate parses, registers and validates as before; +a non-string in these slots keeps its existing refusal at `registerFlow` and +`objectstack validate`; `edges[].condition` and a node's `config.condition` keep +their own rule and sentence (`EVALUATED_EXPRESSION_SOURCE_REQUIRED`); and +`AutomationEngine.evaluateCondition` still answers a blank predicate `false` for +a caller that reaches it directly. The `PREDICATE_SLOT_STRING_REFUSAL` constant +keeps its name and now also names the blank string, so code matching the +constant rather than a copy of its text is unaffected. diff --git a/packages/services/service-automation/src/predicate-slot-blank.test.ts b/packages/services/service-automation/src/predicate-slot-blank.test.ts index fe03e040902..1b753befe94 100644 --- a/packages/services/service-automation/src/predicate-slot-blank.test.ts +++ b/packages/services/service-automation/src/predicate-slot-blank.test.ts @@ -19,8 +19,10 @@ * and the flow parse refuses these values itself since this card — so the * refusal this door hands back is the PARSE's: a Zod issue with code `custom`, * anchored at the slot. That is what these pins assert, because it is what a - * caller receives (the `/automation` write doors map exactly that issue to a - * `VALIDATION_ERROR` field). The engine's own ledger pass refuses the same + * caller receives: the `/automation` write doors (`flowDefinitionRefusal` in + * `runtime/src/domains/automation.ts`) answer a Zod refusal from + * `registerFlow` as `400` `VALIDATION_FAILED`, one `details.fields[]` entry + * per issue, keyed by the issue's path. The engine's own ledger pass refuses the same * value through the same `predicateSlotRefusal`, one step later — the same * two-layer shape a blank `edge.condition` has had since #15807. */ From 842a8537069807982800f4cfbd9c22cda06f13dc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 10:12:53 +0000 Subject: [PATCH 5/7] wip: prescribe expression 'false' for a blank decision branch, pin its routing, scope the stored-flow and route sentences Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- .../17493-predicate-slot-blank-refused.md | 32 ++++--- .../src/predicate-slot-blank.test.ts | 91 +++++++++++++++++++ .../automation/flow-node-expression-paths.ts | 8 +- ...low-predicate-slot-blank-string-refused.ts | 36 ++++---- packages/spec/src/migrations/registry.ts | 36 ++++---- 5 files changed, 148 insertions(+), 55 deletions(-) diff --git a/.changeset/17493-predicate-slot-blank-refused.md b/.changeset/17493-predicate-slot-blank-refused.md index 7f756193566..d98e738a53e 100644 --- a/.changeset/17493-predicate-slot-blank-refused.md +++ b/.changeset/17493-predicate-slot-blank-refused.md @@ -1,5 +1,7 @@ --- '@objectstack/spec': minor +'@objectstack/service-automation': minor +'@objectstack/lint': minor --- fix(spec)!: a blank string in a flow node's predicate slot — a `decision` branch `expression`, a screen field `visibleWhen` — is refused at authoring (#17493) @@ -23,26 +25,26 @@ so. It is now refused at those doors — by `FlowSchema.parse` with a `custom` i anchored at the slot (for example `nodes.1.config.conditions.0.expression`), and by `registerFlow` and `objectstack validate` through that same parse — with a message that leads with the published `PREDICATE_SLOT_STRING_REFUSAL` sentence, -the one these slots already answered with for a non-string value. A flow stored -with such a value no longer registers: the boot log carries a -`failed to register flow` warn naming it, and its trigger is not armed. +the one these slots already answered with for a non-string value. Where such a +value already sits, the whole flow is refused: registered from the metadata +registry or `sys_metadata` at boot, it is skipped with a `failed to register +flow` warn naming it while the flows beside it register; a `defineStack({ flows })` +source throws `StackSchemaInvalidError` for the whole stack; an artifact file is +refused whole at load. ## FROM → TO | you wrote | write instead | |:--|:--| -| `conditions: [{ label: 'high', expression: ' ' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` — or drop that branch | -| `fields: [{ name: 'reason', visibleWhen: '' }]` on a `screen` node | the predicate you meant — `visibleWhen: "status == 'rejected'"` — or drop the `visibleWhen` key | - -**One-line fix:** write the predicate, or remove it — drop `visibleWhen` to show -the field unconditionally, or drop the whole decision branch (a branch requires -its `expression`). - -Removing is behaviour-preserving on these two slots: a blank `visibleWhen` was -already read as absent at run time, and a branch with a blank predicate was not -taken. That is not true of a blank structural `condition` on an edge or a node, -where removing the key makes the step unconditional — see the separate -`flow-edge-condition-evaluated-slot-source-required` migration entry. +| `conditions: [{ label: 'high', expression: ' ' }]` on a `decision` node | the predicate you meant — `{ label: 'high', expression: 'record.amount > 10000' }` — or, to keep what the blank did, `expression: 'false'` | +| `fields: [{ name: 'reason', visibleWhen: '' }]` on a `screen` node | the predicate you meant — `visibleWhen: "status == 'rejected'"` — or, to keep what the blank did, drop the `visibleWhen` key | + +**One-line fix:** write the predicate, or keep what the blank did — `'false'` on +a decision branch (the value the blank evaluated to), no `visibleWhen` on a +screen field (a blank one was read as absent). ⚠️ Do not drop a decision's only +branch: the node then routes by its out-edges alone, and the out-edge that branch +labelled is no longer held back. A blank structural `condition` is another case — +see the `flow-edge-condition-evaluated-slot-source-required` migration entry. **Unchanged.** A non-blank predicate parses, registers and validates as before; a non-string in these slots keeps its existing refusal at `registerFlow` and diff --git a/packages/services/service-automation/src/predicate-slot-blank.test.ts b/packages/services/service-automation/src/predicate-slot-blank.test.ts index 1b753befe94..7b6677eddd9 100644 --- a/packages/services/service-automation/src/predicate-slot-blank.test.ts +++ b/packages/services/service-automation/src/predicate-slot-blank.test.ts @@ -31,6 +31,7 @@ import { PREDICATE_SLOT_STRING_REFUSAL } from '@objectstack/spec/automation'; import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec'; import { AutomationEngine } from './engine.js'; +import { registerLogicNodes } from './builtin/logic-nodes.js'; const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any; @@ -134,3 +135,93 @@ describe('registerFlow refuses a blank string in a ledger predicate slot (#17493 }); }); }); + +/** + * The run-preserving fix the refusal, the ADR-0087 entry and the changeset + * prescribe for a blank decision branch — keep the branch, write + * `expression: 'false'` — measured on the real decision executor against the + * run the blank made. The blank can no longer register, so the "before" run + * registers a placeholder and rewrites the stored branch to the blank. + * + * Dropping the branch is NOT that fix: a decision left with no branch is a + * plain gateway (`logic-nodes.ts`), so every out-edge no `condition` or + * `isDefault` gates runs — pinned last, because the prescription warns of it. + */ +describe('a blank decision branch rewritten to `false` runs what the blank ran (#17493)', () => { + type Out = { target: string; label?: string; isDefault?: boolean }; + type Branch = { label: string; expression: string }; + + /** `blankAt` names the branch that carries the blank in the "before" run. */ + const CASES: Record = { + 'its only branch, beside an `isDefault` out-edge': { + branches: [{ label: 'b0', expression: 'false' }], blankAt: 0, + out: [{ target: 'x', label: 'b0' }, { target: 'y', isDefault: true }], + ran: ['start', 'd', 'y'], + }, + 'a branch that is not the last': { + branches: [{ label: 'b0', expression: 'false' }, { label: 'b1', expression: 'true' }], blankAt: 0, + out: [{ target: 'x', label: 'b0' }, { target: 'y', label: 'b1' }, { target: 'z', isDefault: true }], + ran: ['start', 'd', 'y'], + }, + 'its only branch, with no default out-edge': { + branches: [{ label: 'b0', expression: 'false' }], blankAt: 0, + out: [{ target: 'x', label: 'b0' }, { target: 'y' }], + ran: ['start', 'd', 'x', 'y'], + }, + }; + + /** + * With `blankAt`, the branch registers as `'true'` — a placeholder that + * routes differently in every case — and is rewritten to the blank before + * the run; `ran` below proves the rewrite held. + */ + async function runOf(branches: Branch[] | undefined, out: Out[], blankAt?: number) { + const warned: string[] = []; + const logger = { ...silentLogger, warn: (msg: string) => { warned.push(String(msg)); } }; + const engine = new AutomationEngine(logger); + registerLogicNodes(engine, { logger, getService: () => undefined } as never); + engine.registerNodeExecutor({ type: 'mark', async execute() { return { success: true }; } }); + engine.sealNodeTypeVocabulary(); + const parsed = engine.registerFlow('route', { + name: 'route', label: 'Route', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'd', type: 'decision', label: 'D', + ...(branches ? { + config: { conditions: branches.map((b, i) => ({ ...b, ...(i === blankAt ? { expression: 'true' } : {}) })) }, + } : {}), + }, + ...out.map((o) => ({ id: o.target, type: 'mark', label: o.target })), + ], + edges: [ + { id: 'e0', source: 'start', target: 'd' }, + ...out.map((o, i) => ({ id: `e${i + 1}`, source: 'd', ...o })), + ], + }); + if (blankAt !== undefined) { + const stored = (parsed.nodes[1].config as { conditions: Branch[] }).conditions[blankAt]; + stored.expression = ' '; + } + await engine.execute('route', { params: {} } as never); + const [log] = await engine.listRuns('route'); + return { + ran: (log?.steps ?? []).filter((s) => s.status === 'success').map((s) => s.nodeId), + unclaimedWarns: warned.filter((w) => w.includes('no out-edge carries that label')).length, + }; + } + + it.each(Object.entries(CASES))('%s', async (_name, c) => { + const before = await runOf(c.branches, c.out, c.blankAt); + const after = await runOf(c.branches, c.out); + expect(before.ran).toEqual(c.ran); + expect(after).toEqual(before); + }); + + it('dropping a decision\'s only branch is not that fix: every ungated out-edge then runs', async () => { + const c = CASES['its only branch, beside an `isDefault` out-edge']; + expect((await runOf(c.branches, c.out, c.blankAt)).ran).toEqual(['start', 'd', 'y']); + expect((await runOf([], c.out)).ran).toEqual(['start', 'd', 'y', 'x']); + expect((await runOf(undefined, c.out)).ran).toEqual(['start', 'd', 'y', 'x']); + }); +}); diff --git a/packages/spec/src/automation/flow-node-expression-paths.ts b/packages/spec/src/automation/flow-node-expression-paths.ts index 6da92e96af3..a0dc4af22b4 100644 --- a/packages/spec/src/automation/flow-node-expression-paths.ts +++ b/packages/spec/src/automation/flow-node-expression-paths.ts @@ -401,10 +401,10 @@ export function predicateSlotRefusal(value: unknown): { message: string; source: return { message: `${PREDICATE_SLOT_STRING_REFUSAL} Found a string that is blank after trimming, which states no rule. ` - + 'Write the predicate the branch or field was meant to test (e.g. `record.rating >= 4`), or remove it: ' - + 'on a screen field, drop the `visibleWhen` key to show the field unconditionally; on a decision ' - + 'branch, `expression` is required, so drop that branch from `conditions` — a branch whose predicate ' - + 'is blank is never taken.', + + 'Write the predicate the branch or field was meant to test (e.g. `record.rating >= 4`), or keep what ' + + 'the blank did: on a screen field, drop the `visibleWhen` key; on a decision branch, write ' + + '`expression: \'false\'`, the value the blank evaluated to. Not by dropping a decision\'s only branch: ' + + 'the node then routes by its out-edges alone, and the out-edge that branch labelled is no longer held back.', source: value, }; } diff --git a/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts b/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts index 746e47ae814..3a64d169305 100644 --- a/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.flow-predicate-slot-blank-string-refused.ts @@ -8,8 +8,8 @@ import type { SemanticMigration } from '../../types.js'; // `config.condition`), refused by the evaluated-slot rule under // EVALUATED_EXPRESSION_SOURCE_REQUIRED, where removing a blank condition // INVERTS the edge. These slots are declared `z.string()`, are refused under -// PREDICATE_SLOT_STRING_REFUSAL, and removing the blank is behaviour- -// preserving — a different prescription, which one entry cannot carry for both. +// PREDICATE_SLOT_STRING_REFUSAL, and keep their run with `'false'` or no +// `visibleWhen` — a different prescription, which one entry cannot carry for both. // // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code // span already, and a nested backtick would close it. @@ -21,17 +21,17 @@ export const entry: SemanticMigration = { + 'predicate) — authored as a string that is blank after trimming (\'\', \' \', a tab or a ' + 'newline), at any depth including an ADR-0031 region body. Reachable wherever a flow is ' + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' - + 'objectstack validate, a POST /flows body, and a flow row already sitting in sys_metadata', + + 'objectstack validate, and a flow row already sitting in sys_metadata', replacement: 'the predicate the branch or field was meant to test, as non-blank bare CEL text ' - + '(`expression: \'record.amount > 10\'`, `visibleWhen: \'amount > 0\'`); or REMOVE the blank. ' + + '(`expression: \'record.amount > 10\'`, `visibleWhen: \'amount > 0\'`); or KEEP what the blank did. ' + 'On a screen field, drop the `visibleWhen` key: an absent `visibleWhen` shows the field ' + 'unconditionally, which is what a blank one already did at run time (the resume contract ' + 'treated it as absent, and the renderer fell back to showing the field). On a decision ' - + 'branch, drop the whole `conditions[i]` element: `expression` is required by ' - + '`DecisionConditionSchema`, so a branch cannot keep its label without it, and a branch whose ' - + 'predicate was blank was never taken, so dropping it changes no run. ⚠️ Removal is ' - + 'behaviour-preserving HERE, unlike on a structural condition, where dropping a blank ' + + 'branch, write `expression: \'false\'`: the evaluator answered the blank `false`, so the branch ' + + 'keeps its label and is still never taken. ⚠️ Not by dropping a decision\'s only branch: with ' + + 'no `conditions` the node routes by its out-edges alone, so the out-edge that branch labelled ' + + 'is no longer held back. On a structural condition removal differs again: dropping a blank ' + '`condition` turns a never-firing edge into an always-firing one ' + '(`flow-edge-condition-evaluated-slot-source-required`)', reason: @@ -48,16 +48,16 @@ export const entry: SemanticMigration = { + 'three through `predicateSlotRefusal`, leading with `PREDICATE_SLOT_STRING_REFUSAL`. ' + '⚠️ No D2 conversion, and the reason is the judgment this entry delegates: the blank is ' + 'where an author meant to write a rule, and the platform cannot tell a predicate somebody ' - + 'forgot from one they meant to delete. Removing it preserves what ran; writing it is what ' - + 'the author intended; only the author knows which. ' - + '⚠️ A flow ALREADY STORED with such a blank no longer registers at all, not just that ' - + 'branch or field: `registerFlow` parses through `canonicalizeStoredFlow` → ' - + '`FlowSchema.parse`, and each boot path in `service-automation/src/plugin.ts` logs one ' - + '`warn` naming the flow and continues — its trigger is never armed. ' + + 'forgot from one they meant to delete. Keeping what ran is mechanical; writing the predicate ' + + 'is what the author intended; only the author knows which. ' + + '⚠️ Where such a blank already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not ' + + 'armed, while the flows beside it register; a `defineStack({ flows })` source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + 'ADR-0087, ADR-0032.', acceptanceCriteria: - 'Grep every flow node in `defineStack({ flows })` sources, exported stacks, `POST /flows` ' - + 'bodies and every flow row in `sys_metadata` — including nodes inside a `loop` / ' + 'Grep every flow node in `defineStack({ flows })` sources, exported stacks and every flow ' + + 'row in `sys_metadata` — including nodes inside a `loop` / ' + '`parallel` / `try_catch` region body — for a `decision` node whose ' + '`config.conditions[i].expression`, or a `screen` node whose `config.fields[i].visibleWhen`, ' + 'is a string that is empty after trimming. Each refusal names the node and the branch or ' @@ -67,8 +67,8 @@ export const entry: SemanticMigration = { + 'path; `validateStackExpressions` phrases it as ' + '`node \'check\' (decision) decision branch expression at config.conditions[0].expression`. ' + 'For each hit decide, per the `replacement` note, whether to write the predicate or to ' - + 'remove it — and on a decision branch removing means the whole branch. Two proofs. (1) For ' - + 'a stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and ' + + 'keep what the blank did. Two proofs. (1) For a stack authored in config files, ' + + '`objectstack validate` is clean. (2) Boot the stack and ' + 'confirm each flow REGISTERS: no `failed to register flow` warn for it (the three boot ' + 'paths spell it `[Automation] failed to register flow`, `[Automation] flow re-sync: failed ' + 'to register flow` and `[Automation] cold-boot flow bind: failed to register flow`) — that ' diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index e0a9f77a8df..4d7adfeec6f 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -9064,8 +9064,8 @@ const step18: MigrationStep = { // `config.condition`), refused by the evaluated-slot rule under // EVALUATED_EXPRESSION_SOURCE_REQUIRED, where removing a blank condition // INVERTS the edge. These slots are declared `z.string()`, are refused under - // PREDICATE_SLOT_STRING_REFUSAL, and removing the blank is behaviour- - // preserving — a different prescription, which one entry cannot carry for both. + // PREDICATE_SLOT_STRING_REFUSAL, and keep their run with `'false'` or no + // `visibleWhen` — a different prescription, which one entry cannot carry for both. // // No backticks in `surface` — build-upgrade-guide.ts renders it inside a code // span already, and a nested backtick would close it. @@ -9077,17 +9077,17 @@ const step18: MigrationStep = { + 'predicate) — authored as a string that is blank after trimming (\'\', \' \', a tab or a ' + 'newline), at any depth including an ADR-0031 region body. Reachable wherever a flow is ' + 'authored or stored: defineStack({ flows }) sources, defineFlow(), an exported stack passed to ' - + 'objectstack validate, a POST /flows body, and a flow row already sitting in sys_metadata', + + 'objectstack validate, and a flow row already sitting in sys_metadata', replacement: 'the predicate the branch or field was meant to test, as non-blank bare CEL text ' - + '(`expression: \'record.amount > 10\'`, `visibleWhen: \'amount > 0\'`); or REMOVE the blank. ' + + '(`expression: \'record.amount > 10\'`, `visibleWhen: \'amount > 0\'`); or KEEP what the blank did. ' + 'On a screen field, drop the `visibleWhen` key: an absent `visibleWhen` shows the field ' + 'unconditionally, which is what a blank one already did at run time (the resume contract ' + 'treated it as absent, and the renderer fell back to showing the field). On a decision ' - + 'branch, drop the whole `conditions[i]` element: `expression` is required by ' - + '`DecisionConditionSchema`, so a branch cannot keep its label without it, and a branch whose ' - + 'predicate was blank was never taken, so dropping it changes no run. ⚠️ Removal is ' - + 'behaviour-preserving HERE, unlike on a structural condition, where dropping a blank ' + + 'branch, write `expression: \'false\'`: the evaluator answered the blank `false`, so the branch ' + + 'keeps its label and is still never taken. ⚠️ Not by dropping a decision\'s only branch: with ' + + 'no `conditions` the node routes by its out-edges alone, so the out-edge that branch labelled ' + + 'is no longer held back. On a structural condition removal differs again: dropping a blank ' + '`condition` turns a never-firing edge into an always-firing one ' + '(`flow-edge-condition-evaluated-slot-source-required`)', reason: @@ -9104,16 +9104,16 @@ const step18: MigrationStep = { + 'three through `predicateSlotRefusal`, leading with `PREDICATE_SLOT_STRING_REFUSAL`. ' + '⚠️ No D2 conversion, and the reason is the judgment this entry delegates: the blank is ' + 'where an author meant to write a rule, and the platform cannot tell a predicate somebody ' - + 'forgot from one they meant to delete. Removing it preserves what ran; writing it is what ' - + 'the author intended; only the author knows which. ' - + '⚠️ A flow ALREADY STORED with such a blank no longer registers at all, not just that ' - + 'branch or field: `registerFlow` parses through `canonicalizeStoredFlow` → ' - + '`FlowSchema.parse`, and each boot path in `service-automation/src/plugin.ts` logs one ' - + '`warn` naming the flow and continues — its trigger is never armed. ' + + 'forgot from one they meant to delete. Keeping what ran is mechanical; writing the predicate ' + + 'is what the author intended; only the author knows which. ' + + '⚠️ Where such a blank already sits the whole flow is refused: registered from the metadata ' + + 'registry or `sys_metadata` at boot it is skipped with a `warn` naming it, its trigger not ' + + 'armed, while the flows beside it register; a `defineStack({ flows })` source throws ' + + '`StackSchemaInvalidError` for the whole stack; an artifact file is refused whole at load. ' + 'ADR-0087, ADR-0032.', acceptanceCriteria: - 'Grep every flow node in `defineStack({ flows })` sources, exported stacks, `POST /flows` ' - + 'bodies and every flow row in `sys_metadata` — including nodes inside a `loop` / ' + 'Grep every flow node in `defineStack({ flows })` sources, exported stacks and every flow ' + + 'row in `sys_metadata` — including nodes inside a `loop` / ' + '`parallel` / `try_catch` region body — for a `decision` node whose ' + '`config.conditions[i].expression`, or a `screen` node whose `config.fields[i].visibleWhen`, ' + 'is a string that is empty after trimming. Each refusal names the node and the branch or ' @@ -9123,8 +9123,8 @@ const step18: MigrationStep = { + 'path; `validateStackExpressions` phrases it as ' + '`node \'check\' (decision) decision branch expression at config.conditions[0].expression`. ' + 'For each hit decide, per the `replacement` note, whether to write the predicate or to ' - + 'remove it — and on a decision branch removing means the whole branch. Two proofs. (1) For ' - + 'a stack authored in config files, `objectstack validate` is clean. (2) Boot the stack and ' + + 'keep what the blank did. Two proofs. (1) For a stack authored in config files, ' + + '`objectstack validate` is clean. (2) Boot the stack and ' + 'confirm each flow REGISTERS: no `failed to register flow` warn for it (the three boot ' + 'paths spell it `[Automation] failed to register flow`, `[Automation] flow re-sync: failed ' + 'to register flow` and `[Automation] cold-boot flow bind: failed to register flow`) — that ' From b48aa50ba21ba4830e43f9cc1d86301690a098cf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 10:25:14 +0000 Subject: [PATCH 6/7] wip(service-automation): word the drop-contrast pin as the shipped text does Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- .../service-automation/src/predicate-slot-blank.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/services/service-automation/src/predicate-slot-blank.test.ts b/packages/services/service-automation/src/predicate-slot-blank.test.ts index 7b6677eddd9..8b0f8bcbe00 100644 --- a/packages/services/service-automation/src/predicate-slot-blank.test.ts +++ b/packages/services/service-automation/src/predicate-slot-blank.test.ts @@ -144,8 +144,9 @@ describe('registerFlow refuses a blank string in a ledger predicate slot (#17493 * registers a placeholder and rewrites the stored branch to the blank. * * Dropping the branch is NOT that fix: a decision left with no branch is a - * plain gateway (`logic-nodes.ts`), so every out-edge no `condition` or - * `isDefault` gates runs — pinned last, because the prescription warns of it. + * plain gateway (`logic-nodes.ts`) that routes by its out-edges alone, so the + * out-edge the branch labelled is no longer held back — pinned last, because + * the prescription warns of it. */ describe('a blank decision branch rewritten to `false` runs what the blank ran (#17493)', () => { type Out = { target: string; label?: string; isDefault?: boolean }; @@ -218,7 +219,7 @@ describe('a blank decision branch rewritten to `false` runs what the blank ran ( expect(after).toEqual(before); }); - it('dropping a decision\'s only branch is not that fix: every ungated out-edge then runs', async () => { + it('dropping a decision\'s only branch is not that fix: the out-edge it labelled then runs', async () => { const c = CASES['its only branch, beside an `isDefault` out-edge']; expect((await runOf(c.branches, c.out, c.blankAt)).ran).toEqual(['start', 'd', 'y']); expect((await runOf([], c.out)).ran).toEqual(['start', 'd', 'y', 'x']); From 58a65d12819cc4bc3a690a9207e3074e805303c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 10:53:43 +0000 Subject: [PATCH 7/7] wip: keep the boot warn literal on one changeset line so it greps Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude --- .changeset/17493-predicate-slot-blank-refused.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/17493-predicate-slot-blank-refused.md b/.changeset/17493-predicate-slot-blank-refused.md index d98e738a53e..a05ca2f4367 100644 --- a/.changeset/17493-predicate-slot-blank-refused.md +++ b/.changeset/17493-predicate-slot-blank-refused.md @@ -27,10 +27,10 @@ by `registerFlow` and `objectstack validate` through that same parse — with a message that leads with the published `PREDICATE_SLOT_STRING_REFUSAL` sentence, the one these slots already answered with for a non-string value. Where such a value already sits, the whole flow is refused: registered from the metadata -registry or `sys_metadata` at boot, it is skipped with a `failed to register -flow` warn naming it while the flows beside it register; a `defineStack({ flows })` -source throws `StackSchemaInvalidError` for the whole stack; an artifact file is -refused whole at load. +registry or `sys_metadata` at boot, it is skipped with a +`failed to register flow` warn naming it while the flows beside it register; a +`defineStack({ flows })` source throws `StackSchemaInvalidError` for the whole +stack; an artifact file is refused whole at load. ## FROM → TO