Skip to content

Commit 4ecfd2b

Browse files
claude[bot]claude
andauthored
fix(lint): refuse a blank structural condition at author time, by the rule registerFlow already applies (#17495) (#17665)
`validateStackExpressions` — the pass behind `objectstack validate` — reported NOTHING for a `config.condition` that is blank after trimming, at either structural node slot (a `start` node's trigger gate, a `decision` node's predicate) and on an edge. #17322 rebound `AutomationEngine.registerFlow` to the edge door's non-blank rule, so the same value now stops the flow registering at boot behind a single `warn` line — an author ran validate, got a clean bill, deployed, and the trigger was never armed. `checkStructuralCondition` gains the same second gate the engine's copy has, in the same position (after the shape refusal, before the CEL pass), asking the same IMPORTED schema: `EvaluatedExpressionInputSchema`, the rule `FlowEdgeSchema.condition` has composed since #15807. Not a second hand-written notion of "blank" — that is the drift #15662 built one shared refusal to prevent — so the sentence is the spec's published constant and the three doors cannot answer differently. Two pins re-judged IN PLACE, with the reason recorded, never deleted: * `validate-expressions.test.ts` — "a whitespace-only STRING is untouched — ruled correct, not a defect" recorded that author time and run time AGREED about the blank. #15807 and #17322 removed that ground; the case is flipped and says so, and still pins that the blank refusal and #15662's shape refusal stayed distinct. * `lint-flow-patterns.test.ts` — the `flow-inert-node-condition` zero still stands (that rule is about a key nothing reads, and has no opinion on blankness), but it was readable as "validate says nothing about a blank condition". A cross-site assertion now pins where the refusal actually lives, so the zero cannot be read as silence again. Measured on `d46deba195`, same probe, both structural node slots at once — before: blank `' '` 0, `''` 0; after: 2 and 2. Controls unmoved and proving the probe reaches both slots: brace trap 2, `ast`-only envelope 2, valid CEL 0. Closes #17495 Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU Co-authored-by: claude <noreply@anthropic.com>
1 parent 7880c18 commit 4ecfd2b

4 files changed

Lines changed: 318 additions & 4 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
fix(lint)!: `objectstack validate` refuses a blank structural `condition`, the rule `registerFlow` has carried since #17322 (#17495)
6+
7+
<!-- adr-0087: not-required (already-registered flow-edge-condition-evaluated-slot-source-required) this is the AUTHOR-TIME face of the decision that entry already carries — an evaluated slot requires a non-blank `source`, refused with EVALUATED_EXPRESSION_SOURCE_REQUIRED — reached by importing the same schema rather than deriving a second rule. No key is renamed, retired or given a new meaning here, and no stored document needs rewriting that the entry does not already prescribe. ⚠️ That entry's `surface` / `acceptanceCriteria` name only `edges[].condition` and still want widening to `config.condition`; the registry file is in packages/spec, outside this card's surface, and is already filed as #17493. -->
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):
12+
`validateStackExpressions` — the pass behind `objectstack validate` — now
13+
reports an `error` for a structural `condition` whose source is blank after
14+
trimming. It reported nothing at all before.
15+
16+
The value was already refused by two of the three doors. `FlowEdgeSchema.condition`
17+
composes `EvaluatedExpressionInputSchema` (#15807), so `' '` on an edge is
18+
refused at `FlowSchema.parse`; #17322 rebound `AutomationEngine.registerFlow` to
19+
that same rule, so the same value on a node's `config.condition` stops the flow
20+
registering. `objectstack validate` was the door that still said nothing — so an
21+
author got a clean bill, deployed, and the flow never registered: each boot path
22+
in `service-automation`'s plugin wraps `registerFlow` in `try`/`catch`, logs one
23+
`warn` naming the flow, and continues. On a `start` node that key is the
24+
**trigger gate**, so the whole flow is armed by nothing.
25+
26+
FROM → TO, for a build that used to pass and now fails:
27+
28+
```yaml
29+
# FROM — validate said nothing; registerFlow refuses it at boot
30+
nodes:
31+
- { id: gate, type: start, config: { objectName: lead, triggerType: record-after-update, condition: ' ' } }
32+
- { id: branch, type: decision, config: { condition: { dialect: cel, source: ' ' } } }
33+
34+
# TO — either write the predicate you meant…
35+
nodes:
36+
- { id: gate, type: start, config: { objectName: lead, triggerType: record-after-update, condition: 'record.active == true' } }
37+
- { id: branch, type: decision, config: { condition: { dialect: cel, source: 'record.rating >= 4' } } }
38+
39+
# …or drop the key. An ABSENT condition is still not a malformed one: a start
40+
# node with no `condition` is an ungated trigger, and that is unchanged.
41+
```
42+
43+
The refusal is the edge door's own sentence, not a second one — the finding
44+
carries `EVALUATED_EXPRESSION_SOURCE_REQUIRED` verbatim, located at the node and
45+
slot the author wrote (`flow 'f' · node 'gate' (start) condition`), because all
46+
three doors now ask one imported schema.
47+
48+
Unchanged, deliberately: the **evaluator**. A condition already stored blank
49+
still answers `false` at run time — #15662's ruling on that half stands. What
50+
moved is that it can no longer be authored past validate.

packages/lint/src/lint-flow-patterns.test.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import { TimeRelativeTriggerSchema, LoopConfigSchema, ParallelConfigSchema, TryC
55
// [#5659] The shared identity reduction, asserted beside the rule that consumes
66
// it — the rule's verdict and the drivers' verdict are one object now.
77
import { reduceFilterVerdict } from '@objectstack/spec/data';
8+
// [#17495] The published sentence the blank-condition refusal leads with,
9+
// asserted from the spec's own export rather than re-spelled here — see the
10+
// re-judged pin in `flow-inert-node-condition` below.
11+
import { EVALUATED_EXPRESSION_SOURCE_REQUIRED } from '@objectstack/spec';
812
import { AUTHORING_RULES } from './authoring-rules.js';
913
import {
1014
lintFlowPatterns,
@@ -28,6 +32,10 @@ import {
2832
FLOW_LOOP_BODY_UNCONTAINED,
2933
FLOW_TRY_CATCH_WITHOUT_CATCH,
3034
} from './lint-flow-patterns.js';
35+
// [#17495] Cross-site pin only — the re-judged `flow-inert-node-condition`
36+
// case below. This family's own coverage is unaffected by it;
37+
// `validate-expressions.test.ts` owns the rest of that rule's coverage.
38+
import { validateStackExpressions } from './validate-expressions.js';
3139

3240
const CEL = (source: string) => ({ dialect: 'cel', source });
3341
/**
@@ -1274,9 +1282,31 @@ describe('flow-inert-node-condition (#4414)', () => {
12741282
}).filter((f) => f.rule === FLOW_INERT_NODE_CONDITION)).toHaveLength(0);
12751283
});
12761284

1277-
it('does NOT flag a node with no condition, or an empty one', () => {
1285+
it('does NOT flag a node with no condition, or an empty one — but validate no longer stays silent on the blank (#17495)', () => {
12781286
expect(lintFlowPatterns(conditionNodeFlow('decision', {}))).toHaveLength(0);
12791287
expect(lintFlowPatterns(conditionNodeFlow('decision', { condition: ' ' }))).toHaveLength(0);
1288+
// RE-JUDGED IN PLACE (#17495), not deleted — the two zeros above are
1289+
// deliberate and they still stand, but the REASON the blank one stands has
1290+
// changed and the pin now says so.
1291+
//
1292+
// What it recorded: this pin sat over the whole `lintFlowPatterns` answer,
1293+
// taken at a time when `objectstack validate` as a whole admitted a blank
1294+
// `config.condition` — `registerFlow` admitted it too, and #15662 ruled
1295+
// that agreement correct. It was therefore readable as "validate says
1296+
// nothing about a blank condition". Since #17322 rebound `registerFlow` to
1297+
// the edge door's non-blank rule, that reading would be a false record.
1298+
//
1299+
// What it records now: `flow-inert-node-condition` (#4414) is about a key
1300+
// NOTHING READS, and a blank condition is not that — this family has no
1301+
// opinion on blankness and never had one, so its zero is unchanged and is
1302+
// NOT a statement about validate's answer. The refusal lives one pass over,
1303+
// in `validateStackExpressions` (#17495), and the cross-site assertion
1304+
// below is what keeps the zero above from being read as silence again.
1305+
const blank = conditionNodeFlow('decision', { condition: ' ' });
1306+
const refusals = validateStackExpressions(blank);
1307+
expect(refusals).toHaveLength(1);
1308+
expect(refusals[0].message).toBe(EVALUATED_EXPRESSION_SOURCE_REQUIRED);
1309+
expect(refusals[0].where).toContain("node 'n' (decision) condition");
12801310
});
12811311

12821312
it('does NOT flag a PLUGIN node type — its executor may legitimately read it', () => {

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

Lines changed: 179 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2923,6 +2923,14 @@ describe('validateStackExpressions — reads only keys the spec declares (meta-t
29232923
// `source` receivers for the reason the entry above it records: a local
29242924
// called `message` here would be excused into masking a genuine read.
29252925
'shapeRefusal',
2926+
// [#17495] The blank-source half of the same structural refusal, and the
2927+
// same argument one line up: its keys are that helper's own
2928+
// `{ message, source }`, never metadata keys, and it is named to stay
2929+
// clear of the `message` / `source` receivers. The `safeParse` result it
2930+
// reads reuses the already-excused `verdict` name above — its keys there
2931+
// are `success` / `error`, SafeParseResult's own, so that excuse covers
2932+
// both locals for one reason and masks no metadata read either.
2933+
'blankRefusal',
29262934
// [#14089] NOT a receiver at all — the tail of the `'./flow-variable-scope.js'`
29272935
// import specifier, which this scan cannot tell from `scope.j…`. The two
29282936
// entries above it in this set (`fields`, `guards`) are the same artefact
@@ -3943,9 +3951,29 @@ describe('structural condition shape (#15662)', () => {
39433951
expect(condIssues({ edgeCondition: 'record.rating >= 4' }, "edge 'e1'")).toHaveLength(0);
39443952
});
39453953

3946-
it('a whitespace-only STRING is untouched — ruled correct, not a defect', () => {
3947-
expect(condIssues({ decisionCondition: ' ' }, "node 'branch'")).toHaveLength(0);
3948-
expect(condIssues({ edgeCondition: ' ' }, "edge 'e1'")).toHaveLength(0);
3954+
it('a whitespace-only STRING is refused now (#17495) — the ruling MOVED, it was not deleted', () => {
3955+
// FLIPPED from "a whitespace-only STRING is untouched — ruled correct,
3956+
// not a defect". The pin was deliberate and it is kept in place, flipped,
3957+
// rather than removed: #15662 ruled the blank string correct on the
3958+
// ground that author time and run time AGREED about it — both admitted
3959+
// it. #15807 removed that ground at the edge door
3960+
// (`FlowEdgeSchema.condition` refuses a blank source at
3961+
// `FlowSchema.parse`), #17322 rebound the node door to the same rule at
3962+
// `registerFlow`, and #17495 is this pass catching up: a value the
3963+
// runtime refuses must not pass `objectstack validate`.
3964+
//
3965+
// It is NOT #15662's shape refusal that answers — a string is still a
3966+
// well-shaped condition — so this also pins that the two refusals stayed
3967+
// distinct, exactly as the registration-side pin does.
3968+
const node = condIssues({ decisionCondition: ' ' }, "node 'branch'");
3969+
expect(node).toHaveLength(1);
3970+
expect(node[0].severity).toBe('error');
3971+
expect(node[0].message).toBe(EVALUATED_EXPRESSION_SOURCE_REQUIRED);
3972+
expect(node[0].message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(false);
3973+
expect(condIssues({ edgeCondition: ' ' }, "edge 'e1'")).toHaveLength(1);
3974+
// The EVALUATOR half of #15662's ruling is untouched and out of scope
3975+
// here: a stored blank condition still answers `false`. What changed is
3976+
// that it can no longer be authored past this pass.
39493977
});
39503978

39513979
it('a clean bare-CEL condition still passes', () => {
@@ -3954,6 +3982,154 @@ describe('structural condition shape (#15662)', () => {
39543982
});
39553983
});
39563984

3985+
/**
3986+
* [#17495] The blank structural condition — `objectstack validate`'s half of
3987+
* the refusal #17322 landed in `registerFlow`.
3988+
*
3989+
* The gap this closes, measured on `main` at `d46deba195` with the runtime half
3990+
* already in: `validateStackExpressions` returned **0** for `condition: ' '`
3991+
* carried at BOTH structural slots (a start node's trigger gate and a decision
3992+
* node's predicate), and **0** for `''` — while `registerFlow` threw on the
3993+
* same value. An author ran validate, got a clean bill, deployed, and the flow
3994+
* never registered: each of the three boot paths in
3995+
* `service-automation/src/plugin.ts` wraps `registerFlow` in `try`/`catch`,
3996+
* logs one `warn` naming the flow, and continues. On a `start` node that key is
3997+
* the TRIGGER GATE, so the whole flow is armed by nothing.
3998+
*
3999+
* ⚠️ The rule is IMPORTED, never restated: `EvaluatedExpressionInputSchema` is
4000+
* the edge door's own schema (`FlowEdgeSchema.condition` composes it since
4001+
* #15807) and the one `registerFlow` asks, so the sentence below comes from the
4002+
* spec's published constant and the two doors cannot drift apart. A second
4003+
* hand-written notion of "blank" is precisely what the #15662 campaign built
4004+
* one shared refusal to prevent.
4005+
*/
4006+
describe('blank structural condition (#17495)', () => {
4007+
const objects = [
4008+
{ name: 'crm_lead', fields: { rating: { type: 'number' }, status: { type: 'text' } } },
4009+
];
4010+
4011+
const flowWith = (opts: { startCondition?: unknown; decisionCondition?: unknown; edgeCondition?: unknown }) => ({
4012+
objects,
4013+
flows: [{
4014+
name: 'gate_flow',
4015+
nodes: [
4016+
{
4017+
id: 'start', type: 'start',
4018+
config: {
4019+
objectName: 'crm_lead',
4020+
...('startCondition' in opts ? { condition: opts.startCondition } : {}),
4021+
},
4022+
},
4023+
{
4024+
id: 'branch', type: 'decision',
4025+
config: { ...('decisionCondition' in opts ? { condition: opts.decisionCondition } : {}) },
4026+
},
4027+
],
4028+
edges: [{
4029+
id: 'e1', source: 'start', target: 'branch',
4030+
...('edgeCondition' in opts ? { condition: opts.edgeCondition } : {}),
4031+
}],
4032+
}],
4033+
});
4034+
4035+
/** Both structural NODE slots at once — the two the gap was measured over. */
4036+
const bothSlots = (condition: unknown) =>
4037+
validateStackExpressions(flowWith({ startCondition: condition, decisionCondition: condition }));
4038+
4039+
/**
4040+
* ⭐ The control trio, without which every zero below is void. Each drives
4041+
* the SAME probe over the SAME two slots with a value that is not blank, and
4042+
* each is answered by a DIFFERENT pass — so between them they prove the probe
4043+
* reaches the start node's trigger gate and the decision node's predicate,
4044+
* and that a clean condition is still clean.
4045+
*/
4046+
describe('CONTROLS — the probe reaches both slots and still discriminates', () => {
4047+
it('RED CONTROL — the brace trap reports 2, one per slot (the CEL pass)', () => {
4048+
const control = bothSlots('{record.rating} >= 4');
4049+
expect(control).toHaveLength(2);
4050+
expect(control.map((i) => i.where.includes("node 'start'"))).toContain(true);
4051+
expect(control.map((i) => i.where.includes("node 'branch'"))).toContain(true);
4052+
for (const issue of control) expect(issue.message).toContain('template brace');
4053+
});
4054+
4055+
it('RED CONTROL — an `ast`-only envelope reports 2, one per slot (the SHAPE pass)', () => {
4056+
const control = bothSlots({ dialect: 'cel', ast: { kind: 'const', value: true } });
4057+
expect(control).toHaveLength(2);
4058+
for (const issue of control) {
4059+
expect(issue.message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true);
4060+
}
4061+
});
4062+
4063+
it('GREEN CONTROL — valid CEL on both slots reports 0', () => {
4064+
expect(bothSlots('record.rating >= 4')).toHaveLength(0);
4065+
});
4066+
});
4067+
4068+
it('refuses a blank condition on the START trigger gate and on the DECISION predicate', () => {
4069+
for (const blank of [' ', '']) {
4070+
const issues = bothSlots(blank);
4071+
expect(issues, JSON.stringify(blank)).toHaveLength(2);
4072+
expect(issues.map((i) => i.where)).toEqual([
4073+
"flow 'gate_flow' · node 'start' (start) condition",
4074+
"flow 'gate_flow' · node 'branch' (decision) condition",
4075+
]);
4076+
for (const issue of issues) expect(issue.severity).toBe('error');
4077+
}
4078+
});
4079+
4080+
it('refuses every spelling of blank on the same rule — tabs and newlines included', () => {
4081+
for (const blank of ['\t', '\n', ' \t\n ']) {
4082+
expect(bothSlots(blank), JSON.stringify(blank)).toHaveLength(2);
4083+
}
4084+
});
4085+
4086+
it('refuses a blank `source` INSIDE an envelope too — one seam, two keys, one rule', () => {
4087+
expect(bothSlots({ dialect: 'cel', source: ' ' })).toHaveLength(2);
4088+
// No dialect is still CEL at this slot (#4336), and still blank.
4089+
expect(bothSlots({ source: '' })).toHaveLength(2);
4090+
});
4091+
4092+
it('answers the EDGE door\'s own sentence, not a second one — the two doors do not drift', () => {
4093+
const [issue] = bothSlots(' ');
4094+
// The published constant, asserted from the spec's export rather than
4095+
// re-spelled here: this pass, `FlowSchema.parse` and `registerFlow` all say
4096+
// it because all three ask `EvaluatedExpressionInputSchema`.
4097+
expect(issue.message).toBe(EVALUATED_EXPRESSION_SOURCE_REQUIRED);
4098+
// The author's own text is the attribution, blank though it is.
4099+
expect(issue.source).toBe(' ');
4100+
});
4101+
4102+
it('refuses ONCE — the value-reading passes do not re-report it as an empty condition', () => {
4103+
expect(bothSlots(' ')).toHaveLength(2);
4104+
});
4105+
4106+
it('refuses it on an EDGE condition too — the third structural slot', () => {
4107+
// The edge door refuses this at `FlowSchema.parse` since #15807; on the raw
4108+
// input path `validateStackExpressions` walks, this pass is the gate.
4109+
const issues = validateStackExpressions(flowWith({ edgeCondition: ' ' }));
4110+
expect(issues).toHaveLength(1);
4111+
expect(issues[0].message).toBe(EVALUATED_EXPRESSION_SOURCE_REQUIRED);
4112+
});
4113+
4114+
it('an ABSENT condition is still not a malformed one', () => {
4115+
expect(validateStackExpressions(flowWith({}))).toHaveLength(0);
4116+
expect(bothSlots(undefined)).toHaveLength(0);
4117+
expect(bothSlots(null)).toHaveLength(0);
4118+
});
4119+
4120+
it('a non-blank condition still earns its pre-existing verdict, never the blank one', () => {
4121+
// A shape violation is still the SHAPE refusal: the blank gate runs after
4122+
// it and must not swallow it.
4123+
const shaped = bothSlots(42);
4124+
expect(shaped).toHaveLength(2);
4125+
for (const issue of shaped) {
4126+
expect(issue.message.startsWith(STRUCTURAL_CONDITION_SHAPE_REFUSAL)).toBe(true);
4127+
}
4128+
// And a source that only LOOKS empty to a trimming reader is fine.
4129+
expect(bothSlots(' record.rating >= 4 ')).toHaveLength(0);
4130+
});
4131+
});
4132+
39574133
describe("validateStackExpressions — a non-record entry in an object's `fields:` list (#15742)", () => {
39584134
// `buildFieldIndex` used to cast each member inline
39594135
// (`fields.map(f => (f as AnyRec).name)`), so an empty YAML list item —

0 commit comments

Comments
 (0)