Skip to content

Commit 2c1011b

Browse files
fix(spec)!: refuse a blank string in a flow node's predicate slot — decision branch expression, screen field visibleWhen (#17493) (#19960)
Fixes #17493 Clause-②: no (narrowing) Executes ruling A (`5651023407`). A string that is blank after trimming is refused in two flow-node predicate slots: `decision` `config.conditions[].expression` and `screen` `config.fields[].visibleWhen`. The refusal fires at `FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack validate`, with a message led by `PREDICATE_SLOT_STRING_REFUSAL`. - **Census first** (ruling item 1): at base `3b5607019f`, the dev found no flow in the tree or in the example stacks carrying either blank. The method is in report `5811268231`. - **Code:** - `flow.zod.ts`: a `FlowSchema` refinement over the ledger predicate slots. - `flow-node-expression-paths.ts`: the resolver emits a blank predicate string, and `predicateSlotRefusal` refuses it. - `engine.ts` and `validate-expressions.ts`: comments only. - **Card item 1:** the `structuralConditionRefusal` docblock now records that ruling A answered its open question, and its doors line is corrected for the node slot. - **Card item 2:** a new ADR-0087 entry, `flow-predicate-slot-blank-string-refused`; `registry.ts` is regenerated. - **Pins:** one file per door, plus re-judged existing pins. The dev ablated each refusal red (report `5811268231`). `predicate-slot-blank.test.ts` also pins, on the real decision executor, that `'false'` runs what the blank ran, and that dropping the only branch runs the out-edge it labelled. Both were ablated red (report `5812924875`). - **Seat rulings** (`5811310916`, corrected by `5811904464` and `5812959979`): the parse door stays although `config.condition` has none. On a decision branch, the prescription that keeps the run is `expression: 'false'`, the value the blank evaluated to. Dropping a decision's only branch is named as the thing not to do. Changeset: `@objectstack/spec`, `@objectstack/service-automation` and `@objectstack/lint` at `minor`, plus **BREAKING** (the launch window refuses `major`), with an ADR-0087 `registered` marker. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3b56070 commit 2c1011b

13 files changed

Lines changed: 933 additions & 58 deletions
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/service-automation': minor
4+
'@objectstack/lint': minor
5+
---
6+
7+
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)
8+
9+
Clause-②: no (narrowing)
10+
11+
<!-- adr-0087: registered flow-predicate-slot-blank-string-refused -->
12+
13+
**BREAKING** — an accept-set narrowing on two authored flow-node slots, shipped as
14+
`minor` under the launch-window convention (`check-changeset-no-major` refuses
15+
`major` until GA; breaking-ness is carried by this banner and the ADR-0087
16+
disposition above, not by the level).
17+
18+
**What changed.** A `decision` node's `config.conditions[].expression` and a
19+
`screen` node's `config.fields[].visibleWhen` are declared bare CEL text. A string
20+
that is blank after trimming (`''`, `' '`, a tab or a newline) used to be
21+
accepted there by `FlowSchema.parse`, `AutomationEngine.registerFlow` and
22+
`objectstack validate`, and was then read as "no predicate": the evaluator answers
23+
a blank decision predicate `false`, so that branch was not taken, and nothing said
24+
so. It is now refused at those doors — by `FlowSchema.parse` with a `custom` issue
25+
anchored at the slot (for example `nodes.1.config.conditions.0.expression`), and
26+
by `registerFlow` and `objectstack validate` through that same parse — with a
27+
message that leads with the published `PREDICATE_SLOT_STRING_REFUSAL` sentence,
28+
the one these slots already answered with for a non-string value. Where such a
29+
value already sits, the whole flow is refused: registered from the metadata
30+
registry or `sys_metadata` at boot, it is skipped with a
31+
`failed to register flow` warn naming it while the flows beside it register; a
32+
`defineStack({ flows })` source throws `StackSchemaInvalidError` for the whole
33+
stack; an artifact file is refused whole at load.
34+
35+
## FROM → TO
36+
37+
| you wrote | write instead |
38+
|:--|:--|
39+
| `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'` |
40+
| `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 |
41+
42+
**One-line fix:** write the predicate, or keep what the blank did — `'false'` on
43+
a decision branch (the value the blank evaluated to), no `visibleWhen` on a
44+
screen field (a blank one was read as absent). ⚠️ Do not drop a decision's only
45+
branch: the node then routes by its out-edges alone, and the out-edge that branch
46+
labelled is no longer held back. A blank structural `condition` is another case —
47+
see the `flow-edge-condition-evaluated-slot-source-required` migration entry.
48+
49+
**Unchanged.** A non-blank predicate parses, registers and validates as before;
50+
a non-string in these slots keeps its existing refusal at `registerFlow` and
51+
`objectstack validate`; `edges[].condition` and a node's `config.condition` keep
52+
their own rule and sentence (`EVALUATED_EXPRESSION_SOURCE_REQUIRED`); and
53+
`AutomationEngine.evaluateCondition` still answers a blank predicate `false` for
54+
a caller that reaches it directly. The `PREDICATE_SLOT_STRING_REFUSAL` constant
55+
keeps its name and now also names the blank string, so code matching the
56+
constant rather than a copy of its text is unaffected.

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

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,10 +2235,18 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
22352235
expect(atSlot(['a > 1'])[0].message).toContain('Found an array');
22362236
});
22372237

2238-
it('leaves string predicates alone — including the whitespace-only one', () => {
2239-
// The card states this boundary explicitly so nobody "fixes" it: a
2240-
// whitespace-only STRING is "not authored" on both sides and stays so.
2241-
expect(atSlot(' ')).toHaveLength(0);
2238+
it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => {
2239+
// RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. This
2240+
// pinned `atSlot(' ')` at ZERO findings: "The card states this
2241+
// boundary explicitly so nobody 'fixes' it: a whitespace-only STRING
2242+
// is 'not authored' on both sides and stays so." Both sides do still
2243+
// agree; the ruling is that the agreement is no defence when the
2244+
// author's rule is silently dropped, so the blank is refused here — on
2245+
// the same rule and sentence as the envelope above.
2246+
const blank = atSlot(' ');
2247+
expect(blank).toHaveLength(1);
2248+
expect(blank[0].severity).toBe('error');
2249+
expect(blank[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
22422250
expect(atSlot("lead_record.status == 'converted'")).toHaveLength(0);
22432251
});
22442252
});
@@ -4293,3 +4301,91 @@ describe('masterDetailCount — an unreadable `reference` carrier is refused (#1
42934301
expect(parentScope[0]!.message).toMatch(/declares no `master_detail` relationships/);
42944302
});
42954303
});
4304+
4305+
/**
4306+
* [#17493] (ruling A, 5651023407) — a blank string in a ledger `predicate`
4307+
* slot, at the THIRD door: `objectstack validate`'s expression pass.
4308+
*
4309+
* `decision`'s `config.conditions[].expression` and `screen`'s
4310+
* `config.fields[].visibleWhen` reported NOTHING for `''` / `' '`: the
4311+
* resolver skipped the blank as "not authored", so `validateStackExpressions`
4312+
* never saw it, and a branch carrying it was never taken at run time. The
4313+
* refusal is `predicateSlotRefusal`'s — the spec's one notion, shared with
4314+
* `FlowSchema.parse` and `registerFlow` — so the finding leads with the same
4315+
* published sentence the other two doors answer with.
4316+
*
4317+
* ⚠️ Through the CLI, `objectstack validate` meets these values first at its
4318+
* schema step (`FlowSchema.parse` refuses them there). This pass is what
4319+
* answers for a stack handed to `validateStackExpressions` directly, and it is
4320+
* what these pins drive.
4321+
*/
4322+
describe('a blank string in a ledger predicate slot (#17493)', () => {
4323+
const flowStack = (...middle: Record<string, unknown>[]) => ({
4324+
flows: [{
4325+
name: 'blank_flow',
4326+
nodes: [{ id: 'start', type: 'start' }, ...middle],
4327+
edges: [],
4328+
}],
4329+
});
4330+
const decision = (expression: unknown) =>
4331+
({ id: 'check', type: 'decision', config: { conditions: [{ label: 'Yes', expression }] } });
4332+
const screen = (visibleWhen: unknown) =>
4333+
({ id: 'form', type: 'screen', config: { fields: [{ name: 'amount', type: 'number', visibleWhen }] } });
4334+
const errorsOf = (stack: unknown) =>
4335+
validateStackExpressions(stack as never).filter((i) => (i.severity ?? 'error') === 'error');
4336+
4337+
describe.each(['', ' ', '\t\n '])('the blank %j', (blank) => {
4338+
it('decision branch `config.conditions[].expression` — one error, located at node and branch', () => {
4339+
const found = errorsOf(flowStack(decision(blank)));
4340+
expect(found).toHaveLength(1);
4341+
expect(found[0].severity).toBe('error');
4342+
expect(found[0].where).toBe("flow 'blank_flow' · node 'check' (decision) decision branch expression at config.conditions[0].expression");
4343+
expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
4344+
expect(found[0].source).toBe(blank);
4345+
});
4346+
4347+
it('screen field `config.fields[].visibleWhen` — one error, located at node and field', () => {
4348+
const found = errorsOf(flowStack(screen(blank)));
4349+
expect(found).toHaveLength(1);
4350+
expect(found[0].severity).toBe('error');
4351+
expect(found[0].where).toBe("flow 'blank_flow' · node 'form' (screen) screen field visibleWhen at config.fields[0].visibleWhen");
4352+
expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
4353+
});
4354+
});
4355+
4356+
it('reaches a `decision` inside an ADR-0031 region body', () => {
4357+
const found = errorsOf(flowStack({
4358+
id: 'sweep', type: 'loop',
4359+
config: { collection: '{items}', itemVariable: 'item', body: { nodes: [decision(' ')], edges: [] } },
4360+
}));
4361+
expect(found).toHaveLength(1);
4362+
expect(found[0].where).toContain("loop 'sweep' body");
4363+
expect(found[0].where).toContain('config.conditions[0].expression');
4364+
expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(true);
4365+
});
4366+
4367+
describe('CONTROLS — what this must NOT move', () => {
4368+
it('RED CONTROL — the brace trap on the same slot still earns its own verdict, not the blank one', () => {
4369+
const found = errorsOf(flowStack(decision('{amount} > 1')));
4370+
expect(found).toHaveLength(1);
4371+
expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(false);
4372+
});
4373+
4374+
it('a non-blank predicate and an absent one report nothing', () => {
4375+
expect(errorsOf(flowStack(decision('amount > 1')))).toHaveLength(0);
4376+
expect(errorsOf(flowStack(screen('amount > 0')))).toHaveLength(0);
4377+
expect(errorsOf(flowStack(screen(undefined)))).toHaveLength(0);
4378+
});
4379+
4380+
it('the structural `config.condition` keeps its own rule and sentence', () => {
4381+
const found = errorsOf(flowStack({ id: 'gate', type: 'decision', config: { condition: ' ' } }));
4382+
expect(found).toHaveLength(1);
4383+
expect(found[0].message).toContain(EVALUATED_EXPRESSION_SOURCE_REQUIRED);
4384+
expect(found[0].message.startsWith(PREDICATE_SLOT_STRING_REFUSAL)).toBe(false);
4385+
});
4386+
4387+
it("a `flow-template` slot's blank is untouched", () => {
4388+
expect(errorsOf(flowStack({ id: 'sweep', type: 'loop', config: { collection: ' ' } }))).toHaveLength(0);
4389+
});
4390+
});
4391+
});

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,6 +1167,11 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
11671167
// anything tries to read a source out of it. The refusal is the spec's,
11681168
// shared with the engine's `registerFlow` pass: `error`, because that pass
11691169
// throws, and a shape build refuses must not pass author time.
1170+
//
1171+
// [#17493] The same call refuses a BLANK string too (the resolver emits it
1172+
// for this role). `objectstack validate` meets that value first at its
1173+
// schema step — `FlowSchema.parse` refuses it — so this is the pass that
1174+
// answers for a stack handed to `validateStackExpressions` directly.
11701175
const shapeRefusal = predicateSlotRefusal(raw);
11711176
if (shapeRefusal) {
11721177
issues.push({ where, message: shapeRefusal.message, source: shapeRefusal.source, severity: 'error' });

‎packages/services/service-automation/src/builtin/config-expression-ledger.test.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,28 @@ describe('resolveFlowNodeExpressions — path resolution (#4027)', () => {
281281
expect(found[0].entry.role).toBe('flow-template');
282282
});
283283

284-
it('skips absent and empty values rather than inventing findings', () => {
284+
it('skips absent values rather than inventing findings', () => {
285285
expect(resolveFlowNodeExpressions('screen', {})).toEqual([]);
286286
expect(resolveFlowNodeExpressions('screen', { fields: [] })).toEqual([]);
287-
expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] })).toEqual([]);
288287
// A repeater authored as a non-array must not throw.
289288
expect(resolveFlowNodeExpressions('screen', { fields: 'nope' })).toEqual([]);
290289
});
291290

291+
// RE-JUDGED IN PLACE (#17493, ruling A 5651023407), not deleted. The test
292+
// above used to pin a whitespace-only `visibleWhen` as skipped too — "an
293+
// empty value, not a finding" — on #15572's ground that the resolver and the
294+
// evaluator treated the blank the same way. That agreement was ruled no
295+
// defence: a blank predicate is an author's rule that was never written. So
296+
// the blank is EMITTED for a `predicate` slot, for `registerFlow` and
297+
// `objectstack validate` to refuse, and still skipped for a `flow-template`
298+
// one, which the ruling did not reach.
299+
it('emits a blank string in a predicate slot for the consumer to refuse (#17493)', () => {
300+
expect(resolveFlowNodeExpressions('screen', { fields: [{ visibleWhen: ' ' }] })
301+
.map((f) => [f.path, f.value, f.entry.role]))
302+
.toEqual([['fields[0].visibleWhen', ' ', 'predicate']]);
303+
expect(resolveFlowNodeExpressions('loop', { collection: ' ' })).toEqual([]);
304+
});
305+
292306
// [#15572] A NON-string in a predicate slot is no longer skipped. It was
293307
// skipped as "a type violation for the schema pass to report", and for the
294308
// schemaless node types — `decision` publishes no descriptor `configSchema`,

‎packages/services/service-automation/src/decision-predicate-envelope.test.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,29 @@ describe('decision branch predicate — envelope in a `z.string()` slot (#15572)
102102
});
103103

104104
/**
105-
* The boundary the card states explicitly, so nobody "fixes" it: a
106-
* whitespace-only STRING predicate behaves consistently on both sides —
107-
* "not authored" at the ledger, `false` at the evaluator — and is left
108-
* exactly as it was. Only the envelope shape moved.
105+
* RE-JUDGED IN PLACE (#17493, ruling A 5651023407) — not deleted, because
106+
* the boundary it drew was a real decision and the reason it moved belongs
107+
* next to it.
108+
*
109+
* What it pinned: "The boundary the card states explicitly, so nobody
110+
* 'fixes' it: a whitespace-only STRING predicate behaves consistently on
111+
* both sides — 'not authored' at the ledger, `false` at the evaluator — and
112+
* is left exactly as it was. Only the envelope shape moved." So
113+
* `decisionFlow('str_ws', ' ')` was asserted to register.
114+
*
115+
* Why it moved: the ground is still TRUE — the two sides do agree — and
116+
* was ruled insufficient. A blank branch predicate is an author who meant
117+
* to write a rule; the parser skipping it and the evaluator answering
118+
* `false` only proves the platform did not crash, while the branch is never
119+
* taken and nothing says so. The ruling is the third instance of one rule
120+
* (#17322's `config.condition`, #15811's evaluated `source`), so the blank
121+
* string is now refused here too — by `FlowSchema.parse` inside
122+
* `registerFlow`, under this slot's own sentence. A non-blank string and an
123+
* absent predicate still register, exactly as this test always said.
109124
*/
110-
it('leaves string predicates alone — including the whitespace-only one', () => {
125+
it('leaves non-blank string predicates alone — and refuses the whitespace-only one (#17493)', () => {
111126
expect(() => engine.registerFlow('str_ok', decisionFlow('str_ok', 'record.rating >= 4'))).not.toThrow();
112-
expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', ' '))).not.toThrow();
127+
expect(() => engine.registerFlow('str_ws', decisionFlow('str_ws', ' '))).toThrow(PREDICATE_SLOT_STRING_REFUSAL);
113128
expect(() => engine.registerFlow('str_absent', decisionFlow('str_absent', undefined))).not.toThrow();
114129
});
115130

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9522,6 +9522,14 @@ export class AutomationEngine implements IAutomationService {
95229522
// evaluation must be one set, and the refusal itself is the
95239523
// spec's, shared with `objectstack validate` so build and
95249524
// author time cannot disagree about the shape.
9525+
//
9526+
// [#17493] The same call now refuses a BLANK string too: the
9527+
// resolver emits it for this role, and `predicateSlotRefusal`
9528+
// refuses it under the same sentence. ⚠️ For that value this
9529+
// is the second line, not the first — `FlowSchema.parse`
9530+
// (in `canonicalizeStoredFlow`, above) refuses it before
9531+
// this pass runs, the same way it meets a blank
9532+
// `edge.condition` before `checkStructuralCondition` does.
95259533
const shapeRefusal = predicateSlotRefusal(found.value);
95269534
if (shapeRefusal) {
95279535
failures.push(` • ${slotWhere}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``);

0 commit comments

Comments
 (0)