Skip to content

Commit 92865f6

Browse files
claude[bot]claude
andauthored
fix(service-automation)!: a whitespace-only config.condition is refused at registerFlow (#17322) (#17491)
A flow node's `config.condition` — a `decision` node's predicate, and on a `start` node the trigger gate — accepted a whitespace-only string, passed it to `AutomationEngine.evaluateCondition`, and landed on the empty-source arm (`exprStr.trim() === ''`), which returns `false` under a comment naming that arm as being for an UNAUTHORED condition. The value was authored: the branch never ran, forever, with nothing said at any layer. Since #15807 the EDGE door refuses exactly this value at `FlowSchema.parse` (`FlowEdgeSchema.condition` composes `EvaluatedExpressionInputSchema`). One operation, two implementations, inconsistent behaviour: the governed side wins and the other rebinds. A node's `config` is an open `z.record`, so `registerFlow`'s structural pass is the only producer-side gate it has, and the rule is applied there — by importing `EvaluatedExpressionInputSchema` and running the condition's source through it, so the two doors cannot drift into two notions of "blank" or two sentences for it. Applied to the SOURCE rather than to the whole value: the union would also refuse an envelope with no `dialect` or with a dialect outside its enum, both of which this slot admits, and refusing them would widen the narrowing past what was ruled. `evaluateCondition` and `structuralConditionRefusal` are unchanged. Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW Co-authored-by: Claude <noreply@anthropic.com>
1 parent e7e883c commit 92865f6

3 files changed

Lines changed: 274 additions & 7 deletions

File tree

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
---
2+
"@objectstack/service-automation": minor
3+
---
4+
5+
fix(service-automation)!: a whitespace-only `config.condition` is refused at `registerFlow`, the rule the edge door has carried since #15807 (#17322)
6+
7+
<!-- adr-0087: not-required (already-registered flow-edge-condition-evaluated-slot-source-required) this is a second face of the decision that entry already carries — an evaluated slot requires a non-blank `source`, refused with EVALUATED_EXPRESSION_SOURCE_REQUIRED — applied to the other structural condition slot by importing the same schema rather than by deriving a second rule; no key is renamed, retired or given a new meaning here. ⚠️ That entry's `surface` and `acceptanceCriteria` name only `edges[].condition`, so they need widening to `config.condition` for a consumer replaying the chain; that file is in packages/spec, outside this card's package, and is filed as a follow-up rather than edited here. -->
8+
9+
**BREAKING** in the accept-set sense, landing in the launch window as `minor`
10+
(the lockstep convention: `major` is refused by `check-changeset-no-major`, and
11+
breaking-ness is carried by this banner plus the ADR-0087 disposition): a flow
12+
node's `config.condition` — a `decision` node's predicate, and on a `start` node
13+
the **trigger gate** — is now refused at `registerFlow` when its source is blank
14+
after trimming, where it used to register clean and answer a **silent `false`**
15+
at every evaluation.
16+
17+
Two doors, the same authored value, two fates until now. `FlowEdgeSchema.condition`
18+
composes `EvaluatedExpressionInputSchema` (#15807), so `' '` on an edge is
19+
refused at `FlowSchema.parse`, by name. A node's `config` is an open
20+
`z.record(z.string(), z.unknown())`, so the same value passed through verbatim,
21+
reached `AutomationEngine.evaluateCondition`'s empty-source arm — `exprStr.trim()
22+
=== ''` — and returned `false`, under a comment that names that arm as being for
23+
an **unauthored** condition. `' '` was authored. The branch never ran, forever,
24+
with nothing said at any layer.
25+
26+
```yaml
27+
nodes:
28+
- { id: gate, type: start, config: { objectName: lead, triggerType: record-after-update, condition: ' ' } } # the flow was gated shut
29+
- { id: branch, type: decision, config: { condition: { dialect: cel, source: ' ' } } } # the same blank, through the envelope key
30+
```
31+
32+
> An expression in an evaluated slot needs a non-blank `source`: the expression
33+
> engine evaluates `source` (the canonical persisted form of phase M9.1) and
34+
> cannot evaluate `ast` alone, so an envelope carrying only `ast`, or a `source`
35+
> that is blank after trimming, would validate and register and then fault at
36+
> run time. Write `{ dialect: 'cel', source: '…' }`.
37+
38+
- **The rule is imported, not re-derived.** `registerFlow`'s structural pass runs
39+
the condition's source through `EvaluatedExpressionInputSchema` itself, so the
40+
node door and the edge door cannot drift into two notions of "blank" or two
41+
sentences for it — the property the #15662 campaign built the shared refusal
42+
for. Nothing is exported from this package to carry it, and no new export was
43+
added.
44+
- **Applied to the SOURCE, not to the whole value**, deliberately: the union
45+
would also refuse an envelope with no `dialect` or with a dialect outside its
46+
enum, and this slot admits both (`structuralConditionRefusal`'s docblock,
47+
#4336). The narrowing is exactly the blank population and nothing else — a
48+
`cron` envelope with a real source still earns its own pre-existing verdict,
49+
and a bare string with a `{…}` brace trap still earns #1491's.
50+
- **`evaluateCondition` is unchanged and still answers `false`.** It is the
51+
shared evaluator and a public method on an exported class, so its throw
52+
behaviour is itself a contract; and a stored flow reaches it whatever the
53+
producer refuses. This change is at the producer only.
54+
- **`structuralConditionRefusal` is unchanged.** A string is still a well-shaped
55+
condition; the new refusal sits behind the shape one and in front of the CEL
56+
one, and answers the evaluated-slot sentence rather than
57+
`STRUCTURAL_CONDITION_SHAPE_REFUSAL`.
58+
59+
**What an author does with a refused condition.** A whitespace-only condition was
60+
never a predicate — the engine answered `false`, so the branch never fired, and on
61+
a `start` node the flow never triggered. **Remove the `condition` key** if the node
62+
was meant to be unconditional, or **write the expression** if it was meant to
63+
branch. ⚠️ Those two are not interchangeable: a refused condition never fired,
64+
while an absent `condition` on a decision node is an unconditional branch that
65+
always fires and an absent one on a start node is a gate that always opens.
66+
Deleting the key to clear the refusal inverts the node rather than preserving it.
67+
Every condition with a non-blank source is unchanged, and nothing is renamed or
68+
retired.
69+
70+
**A flow ALREADY STORED in `sys_metadata` stops running entirely — the whole flow,
71+
not just the branch.** Stored flows are deliberately not canonicalized by
72+
`applyConversionsToStoredItem` (`spec/src/conversions/stored.ts`, and the same
73+
skip in `metadata/src/loaders/database-loader.ts`'s `rowToData`); they canonicalize
74+
at `registerFlow`, and each of the three boot paths in
75+
`service-automation/src/plugin.ts` wraps that call in `try`/`catch`, logs one
76+
`warn` naming the flow, and continues. So a node condition that used to answer a
77+
silent `false` while the rest of the flow ran now takes the flow down with it: it
78+
is never registered, its trigger is never armed, and the announcement is that one
79+
warn line — `[Automation] failed to register flow` at boot, `[Automation]
80+
cold-boot flow bind: failed to register flow` at the kernel:ready bind,
81+
`[Automation] flow re-sync: failed to register flow` on a re-sync. The warn line
82+
is also the locator: the refusal names the node and the slot, e.g. `node 'gate'
83+
(start) condition`. A stack authored in config files has a second door,
84+
`objectstack validate` — see the note below for what that door does **not** yet
85+
say.
86+
87+
**A repo-wide census on this branch found zero authored `config.condition` values
88+
of this shape**, against a lit control: a textual probe over all 8,123 tracked
89+
source files found **461** non-blank `condition:` string literals and **zero**
90+
blank-after-trim ones in any authored flow (the four blank hits are two prose
91+
examples inside #15807's own changeset and two `packages/lint` test fixtures).
92+
There is nothing in this repository to rewrite.
93+
94+
⚠️ **Two follow-ups this change does not carry, both outside this card's package.**
95+
(1) The ADR-0087 D3 entry named above,
96+
`flow-edge-condition-evaluated-slot-source-required`, registers the decision this
97+
change is a second face of — an evaluated slot requires a non-blank `source` — but
98+
its `surface` and `acceptanceCriteria` name only `edges[].condition`. They need
99+
widening to `config.condition` so a consumer replaying the chain is told to sweep
100+
the node key too; that file is in `packages/spec`.
101+
(2) `@objectstack/lint`'s `validate-expressions` applies only
102+
`structuralConditionRefusal` to a structural condition, so `objectstack validate`
103+
still reports nothing for a blank `config.condition` that `registerFlow` now
104+
refuses — the two doors disagree until that rule is rebound as well.

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ import { predicateSlotRefusal, resolveFlowNodeExpressions, structuralConditionRe
3535
// ({@link AutomationEngine.valueEnvelopeRefusals}), so a flow that registers can
3636
// never be refused at run time and vice versa.
3737
import { AssignmentValueSchema, ASSIGNMENT_VALUE_ENVELOPE_REFUSAL } from '@objectstack/spec/automation';
38+
// [#17322] The EVALUATED-slot rule, IMPORTED rather than re-derived. It is the
39+
// rule `FlowEdgeSchema.condition` already composes since #15807, so a node's
40+
// `config.condition` — which no schema stands in front of — is held to the same
41+
// notion of "blank" and answers the same published sentence. A second,
42+
// hand-written `trim()` here is exactly the drift the #15662 campaign built the
43+
// shared refusal to prevent. See `checkStructuralCondition` in `registerFlow`.
44+
import { EvaluatedExpressionInputSchema, EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec';
3845
import { applyConversionsToFlow, type ConversionNotice, type ConversionConflictNotice } from '@objectstack/spec';
3946
// [ADR-0126 §7.3] "Does a code package ship this flow?" for the subflow guard.
4047
// Routed through the local precedence module rather than importing
@@ -8060,14 +8067,47 @@ export class AutomationEngine implements IAutomationService {
80608067
* node's trigger gate is read from. Same severity as a malformed
80618068
* predicate (this throws): the reject set of registration and the reject
80628069
* set of evaluation must be one set.
8070+
*
8071+
* [#17322] SECOND gate, after the shape one and before the CEL one: the
8072+
* source must be non-blank. `structuralConditionRefusal` admits every
8073+
* string by design, so a whitespace-only `config.condition` passed here
8074+
* and landed on `evaluateCondition`'s empty-source arm — a SILENT
8075+
* `false`, i.e. a branch that never runs, forever, under a comment that
8076+
* names that arm as being for an UNAUTHORED condition. `' '` was
8077+
* authored, and on a `start` node that key is the trigger gate. Since
8078+
* #15807 the EDGE door refuses exactly this value at
8079+
* `FlowSchema.parse`, and 带治理的一侧胜出,另一侧改绑: the node door,
8080+
* which has no schema in front of it, aligns to the governed side HERE,
8081+
* at the producer.
80638082
*/
8083+
const evaluatedSourceRefusal = (raw: unknown): { message: string; source: string } | undefined => {
8084+
// Reached only after `structuralConditionRefusal` cleared the value,
8085+
// so `raw` is bare text or an envelope carrying a string `source`.
8086+
const source = typeof raw === 'string' ? raw : (raw as { source?: unknown }).source;
8087+
if (typeof source !== 'string') return undefined;
8088+
// The rule and its sentence both come from the edge door's own
8089+
// schema. Applied to the SOURCE rather than to the whole value on
8090+
// purpose: the union would also refuse an envelope with no
8091+
// `dialect` or a dialect outside its enum, both of which this slot
8092+
// admits (`structuralConditionRefusal`'s docblock, and #4336) — and
8093+
// refusing them would widen this narrowing past what was ruled.
8094+
const verdict = EvaluatedExpressionInputSchema.safeParse(source);
8095+
if (verdict.success) return undefined;
8096+
return { message: verdict.error.issues[0]?.message ?? EVALUATED_EXPRESSION_SOURCE_REQUIRED, source };
8097+
};
8098+
80648099
const checkStructuralCondition = (where: string, raw: unknown): void => {
80658100
if (raw == null) return;
80668101
const shapeRefusal = structuralConditionRefusal(raw);
80678102
if (shapeRefusal) {
80688103
failures.push(` • ${where}: ${shapeRefusal.message}\n source: \`${shapeRefusal.source}\``);
80698104
return;
80708105
}
8106+
const blankRefusal = evaluatedSourceRefusal(raw);
8107+
if (blankRefusal) {
8108+
failures.push(` • ${where}: ${blankRefusal.message}\n source: \`${blankRefusal.source}\``);
8109+
return;
8110+
}
80718111
check(where, raw);
80728112
};
80738113

0 commit comments

Comments
 (0)