Repository navigation
Commit 16c5473
Fixes #19961
Clause-②: no (narrowing)
A `decision` branch with no `expression` (the key absent, or
`expression: null`) is now refused at all three doors:
`FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack
validate`. It goes through the same walk, the same function and the same
lead sentence that already refuse a blank branch predicate (#17493 / PR
#19960).
## What was wrong, measured on `origin/main` `a9fb83ef`
`DecisionConditionSchema` declares a branch `{ label, expression }` with
`expression` a required `z.string()`. Nothing parses a decision node's
open `config` against that schema. The expression ledger's resolver also
skipped an absent value as "not authored". So the build accepted a
branch that the run refuses.
| branch | `FlowSchema.parse` | `registerFlow` | `objectstack validate
--json` |
|:--|:--|:--|:--|
| `{ label: 'y' }` (the card's shape) | accepted | registered | `valid:
true`, exit 0 |
| `{ label: 'y', expression: null }` | accepted | registered | `valid:
true`, exit 0 |
| `{ label: 'y', condition: 'true' }` (the edge's spelling) | accepted |
registered | `valid: true`, exit 0 |
| `{ label: 'y', expression: ' ' }` (control, #19960) | refused,
`custom` at `nodes.1.config.conditions.0.expression` | refused, same
issue | `valid: false`, exit 1, same path |
| `{ label: 'y', expression: 'true' }` (control) | accepted | registered
| `valid: true`, exit 0 |
What the run did with it: `evaluateCondition({ dialect: 'cel', source:
undefined })` and `source: null` both throw `condition evaluation error:
A structural condition …`. On the real decision executor, a run that
reaches such a branch ends `success: false` at the branch (pinned
below).
## The fix: one walk, one judge
- `packages/spec/src/automation/flow-node-expression-paths.ts`
- `FlowNodeExpressionPath` gains `required?: true`. It is set on
`decision` `conditions[].expression` and on nothing else.
- For a `required` predicate slot, `resolveFlowNodeExpressions` now
emits the absent or `null` value on a branch that exists. It still skips
a decision with no `conditions`, an empty list, and an absent screen
`visibleWhen`.
- `predicateSlotRefusal(undefined | null)` now has its own detail
sentence and prescription, under the unchanged
`PREDICATE_SLOT_STRING_REFUSAL` lead.
- `packages/spec/src/automation/flow.zod.ts`: the `FlowSchema`
predicate-slot refinement now admits the absent or `null` value of a
`required` slot, next to strings. Every other non-string keeps its
#15572 scope, so it is still not refused at this door.
- `packages/lint/src/validate-expressions.ts`: `checkDeclaredPredicate`
dropped its `raw == null` early return. Whether an absent value is a
finding is the resolver's call. The early return answered "valid" for
the exact value `FlowSchema.parse` refuses, for any caller of
`validateStackExpressions` that does not parse first. This site is
outside the claim's file surface. The measurement put the third door's
refusal there (ablation B below).
- `engine.ts`: no change. Its ledger pass already calls
`predicateSlotRefusal` on everything the resolver emits, and
`registerFlow` parses first, so the parse answers first. That is the
same two-layer shape the blank has.
**The route choice (Zone 2 item 3).** I chose (a), the predicate-slot
walk treating an absent `expression` as a refused slot. I did not choose
(b), parsing each branch against `DecisionConditionSchema`. Reasons, per
axis:
- Business need, measured: the only named producer is objectui's
`rowsToList`, which writes `{ label }`. The absent key is the whole
defect.
- Long-term design: (a) keeps one judge (`predicateSlotRefusal`) and one
walk for the three doors. (b) would be a second judge with Zod's own
messages, a different prescription at each door, and a key-set closure
on branches that nobody ruled.
- Guarding AI authors: both refuse loudly. (a) also names the
`condition` alias mistake in the same prescription.
- No scope growth: (a) touches one ledger entry. (b) would narrow a much
wider accept set, including unknown branch keys and the whole branch
shape.
`DecisionConditionSchema` and the fenced `DecisionConfigSchema` / `mode`
region are untouched. #20168's PR #20279 landed while this was in
flight, and this branch is merged over it (`7534fd7e`). Its refusal
lives in `DecisionConfigSchema`, which no door parses a node's config
against, so it and this walk do not meet. Its suite is green here (in
the `src/automation` run below).
**Prescription wording.** Triage (`5811370954`) says PR #19960's
decision-branch prescription is "删掉这个分支" (delete the branch). The landed
#19960 text says something else: write the predicate, or `expression:
'false'` to keep what the blank ran, and ⚠️ **not** by dropping a
decision's only branch. That clause is pinned by
`predicate-slot-blank.test.ts`. This PR follows the landed wording. It
drops the "keep what ran" half, because an absent predicate never ran:
it failed the run at the branch. So `'false'` is offered as "keep the
branch and its label, never take it", and nothing is claimed to be
preserved.
### The refusal text, quoted (`predicateSlotRefusal(undefined)`, byte
for byte what all three doors print)
```text
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. Found nothing — the key is absent where the slot is required: a decision branch is `{ label, expression }` and its `expression` is not optional, so a branch without one states no rule. Write the predicate the branch was meant to test (e.g. `record.rating >= 4`); a predicate written under another key — `condition` is the edge's spelling — belongs in `expression`. There is no run to keep: the executor evaluates every branch it reaches, and a branch with no `expression` failed the run there. To keep the branch and its label but never take it, write `expression: 'false'`. 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.
```
For `null`, `Found nothing — the key is absent` reads `Found` followed
by the code-spelled `null`. The lead sentence
(`PREDICATE_SLOT_STRING_REFUSAL`) is unchanged, byte for byte.
**After, measured on `e702ebd4` (the real CLI door, spec rebuilt; no
file of this diff changed after that).** `{ label: 'y' }`, `expression:
null` and `condition: 'true'` all give `objectstack validate --json`
`valid: false`, exit 1, one `custom` error at
`flows.0.nodes.1.config.conditions.0.expression`. The absent and alias
messages are byte-identical. `registerFlow` refuses the same three with
a `custom` issue at `nodes.1.config.conditions.0.expression`.
`expression: 'true'` still validates and registers. The blank keeps its
own message.
## Pins: one table per door, the same five rows
Every refused row asserts the issue `code`, the `path`, and the full
message equal to the spec's own `predicateSlotRefusal(value).message`.
-
`packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts`:
`FlowSchema.parse`.
- The five rows: absent, `null`, `condition` alias, blank control, real
accept control.
- Branch index 1 is anchored. The ADR-0031 region body is anchored.
- Controls: a decision with no `conditions` or `[]` still parses; an
absent screen `visibleWhen` still parses.
-
`packages/services/service-automation/src/decision-branch-expression-absent.test.ts`:
`registerFlow`.
- The same table. `getFlow` is `null` after each refusal.
- Region body.
- On the real decision executor: the absent branch failed the run
(`success: false`, `condition evaluation error`, ran `['start']`);
`expression: 'false'` routes to the fallback.
- `packages/lint/src/validate-expressions.test.ts` `describe('a decision
branch with no expression (#19961)')`: `validateStackExpressions`, with
the same table, the exact `where` string, branch index 1, and controls.
- `packages/spec/src/automation/flow-node-expression-paths.test.ts`:
- The resolver emits `undefined` or `null` for the decision slot and
skips everything else.
- `predicateSlotRefusal(undefined | null)` prescription clauses are
pinned by name.
- The `required` set is pinned to exactly
`decision.conditions[].expression (predicate)`, because the absent arm's
wording is decision-specific.
-
`packages/services/service-automation/src/builtin/config-expression-ledger.test.ts`:
the reconciliation ratchet now reads each channel's JSON-Schema
`required` list. It asserts that the ledger's `required` flags equal the
channel's, in both directions, over the `predicate` role. It derives,
not assumes, that `visibleWhen` is optional. It asserts that `required`
is never set on another role. The channels do require `loop.collection`
/ `map.collection`, but no door refuses their absence (reported to the
seat as an out-of-scope finding).
**Pin sweep.** One published pin flipped:
`decision-predicate-envelope.test.ts` asserted
`decisionFlow('str_absent', undefined)` registers. It was re-judged in
place, and the reason is written beside it. It now asserts the throw
carries `PREDICATE_SLOT_STRING_REFUSAL` and `Found nothing — the key is
absent where the slot is required`.
Repo sweep for other branches without an `expression`: a
bracket-balanced scan of every `.ts` / `.json` / `.yaml` file that
mentions both `decision` and `conditions` found only this PR's own
fixtures. A grep of helper-built branches (`{ label: …, expression }`
shorthand) found 4 sites, all in suites run below. No other package's
test builds a `decision` with `conditions`.
## Ablation: the pins can fail
Both ablations were run on committed state through
`scripts/ablation-replace.mjs`, which wraps the change, verifies it on
disk and restores it with a trap. Both proved restore by blob hash equal
to HEAD and an empty `git diff HEAD`.
- **A: the `required` flag neutralised.** `required: true,` was replaced
by a spread that is `{}` unless a `globalThis` flag named
`ABLATION_19961` is set.
- `ablation-dist-preflight.mjs @objectstack/spec ABLATION_19961` found
the marker present in 20 built files.
- Red, in the expected direction:
- spec: 7 failed (the absent, `null` and alias rows, index 1, region,
the `required`-set pin, the resolver pin);
- service-automation: 6 failed (the three rows, region, the re-judged
envelope pin, the ratchet);
- lint: 4 failed.
- The blank and real controls stayed green at every door.
- Restore leg: rebuild, `--absent` marker gone from all 222 built files,
whole-tree `git status` clean. spec 52/52, service-automation 34/34 and
lint 344/344 green.
- The first attempt was a no-op and its reading was discarded: my
replacement was not valid TypeScript, so the transform failed and the
build never ran.
- **B: the lint early return put back** (`if (raw == null) return {
refused: false };`): lint showed 4 failed (absent, `null`, alias, index
1), and the blank and real rows stayed green. Restored by blob hash. The
first attempt was refused by the tool before running anything, because
the anchor matched its own replacement.
## Producer census (Zone 2 item 4): authored count 0
- `examples/**` at `e702ebd4`: 3 flows carry `decision` nodes (app-crm
`convert-lead`, app-showcase `needs_exec` / `triage`, app-todo
`check_recurring`). All of them branch on out-edges and declare no
`conditions`, so 0 branches lack an `expression`.
- `packages/**` non-test: no default flow carries a `decision` node. The
`content/docs/automation/flows.mdx` examples: 3 `conditions` lists, all
with `expression`.
- cloud `origin/main` `96eb092f`: 0 `decision` nodes.
`service-ai-studio`'s authoring whitelist names `decision` as an
authorable node type, so AI-authored flows now meet this refusal.
- objectui at the pin `f8a9d0fb` (`.objectui-sha`):
`FlowObjectListField` `rowsToList` still drops a blank cell, so a branch
row with an empty expression cell is written as `{ label }`. That is the
known writer. Triage accepted that its save now fails loudly, so it is
not fixed here.
## ADR-0087 and changeset
- New semantic entry `flow-decision-branch-expression-absent-refused`
(major 18) and a regenerated `registry.ts`.
- There is no D2 conversion: the platform cannot know the rule the
author left out, and `'false'` would change behaviour rather than keep
it.
- The changeset
`.changeset/19961-decision-branch-expression-absent-refused.md`:
`@objectstack/spec` and `@objectstack/lint` `minor`, BREAKING, with the
FROM → TO table.
- `service-automation` gets no changeset: its diff is test files only,
and those are not in `files[]`.
## Verification (final head `7534fd7e`, which is `origin/main`
`6a6a17b6` merged, #20279 included)
- Tests (`os-verify-lock`, spec rebuilt on this head):
- spec `src/automation` + `src/migrations`: 1005/1005, including
#20279's `schemaless-node-config.test.ts`.
- service-automation: the 4 predicate-slot / ledger files, 50/50.
- lint `validate-expressions.test.ts`: 344/344.
- Full suites, run on the first merge head `266cd043`: spec 16855 passed
(583 files), service-automation 1767/1767, lint 4269/4269.
- Typecheck, including the test layers, on `266cd043`: spec, lint and
service-automation all exit 0. No file of this diff changed after that.
- Gates: `dispatch-gates.mjs --commands` re-derived on `7534fd7e` gives
90 families. 89 ran with exit 0. `dispatch-gates --ran` answers "90
derived famil(ies) accounted for — 89 run, 1 NOT-MEASURED".
- NOT MEASURED: `check:type-check-debt`. Its `--re-measure` runs a
whole-tree `turbo run build --filter=./packages/*` outside
`os-verify-lock`. This diff touches no DEBT-ledger package.
- On earlier heads, two gates needed their prerequisites built first,
and both then exited 0. `check:dual-build-cjs-loads` answered
PREREQUISITE NOT MET because 12 unrelated packages were unbuilt.
`check:dts-closure` went red on local state: 6 packages lost their
`.d.ts` to my own interrupted `--re-measure` build. That is not this
diff.
- `eslint --no-inline-config --format json` over the 12 changed `.ts`
files on `7534fd7e`: 12 files, 0 errors, 0 warnings.
- Population: `eslint.config.mjs` `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']`, and no file here is ignored.
- Invariance: the config never enables type-aware linting (no
`parserOptions.project`), so this diff cannot move a verdict on an
untouched file.
- Declared narrowing: after the second and third `origin/main` merges, I
re-ran the targeted suites above and every gate, not the full package
suites. The incoming commits touch other surfaces (rls, date comparands,
report charts, cli generate, pm scripts, and #20279's
`DecisionConfigSchema` `mode`), not the flow predicate walk.
## Acceptance notes (observed, not filed)
- `service-automation` `engine.ts` `evaluateCondition` still has an
inline comment saying the empty-source arm is where "a `decision` node
whose `conditions[]` entry has no `expression`" lands and answers
`false`. Since #16038 the shape gate throws first, and since this PR the
shape cannot register. The comment is stale; no behaviour follows from
it. Carrier: whoever next edits `evaluateCondition`, else none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent cc40033 commit 16c5473
13 files changed
Lines changed: 777 additions & 23 deletions
File tree
- .changeset
- packages
- lint/src
- services/service-automation/src
- builtin
- spec/src
- automation
- migrations
- entries/semantic
Lines changed: 61 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
4398 | 4399 | | |
4399 | 4400 | | |
4400 | 4401 | | |
| 4402 | + | |
| 4403 | + | |
| 4404 | + | |
| 4405 | + | |
| 4406 | + | |
| 4407 | + | |
| 4408 | + | |
| 4409 | + | |
| 4410 | + | |
| 4411 | + | |
| 4412 | + | |
| 4413 | + | |
| 4414 | + | |
| 4415 | + | |
| 4416 | + | |
| 4417 | + | |
| 4418 | + | |
| 4419 | + | |
| 4420 | + | |
| 4421 | + | |
| 4422 | + | |
| 4423 | + | |
| 4424 | + | |
| 4425 | + | |
| 4426 | + | |
| 4427 | + | |
| 4428 | + | |
| 4429 | + | |
| 4430 | + | |
| 4431 | + | |
| 4432 | + | |
| 4433 | + | |
| 4434 | + | |
| 4435 | + | |
| 4436 | + | |
| 4437 | + | |
| 4438 | + | |
| 4439 | + | |
| 4440 | + | |
| 4441 | + | |
| 4442 | + | |
| 4443 | + | |
| 4444 | + | |
| 4445 | + | |
| 4446 | + | |
| 4447 | + | |
| 4448 | + | |
| 4449 | + | |
| 4450 | + | |
| 4451 | + | |
| 4452 | + | |
| 4453 | + | |
| 4454 | + | |
| 4455 | + | |
| 4456 | + | |
| 4457 | + | |
| 4458 | + | |
| 4459 | + | |
| 4460 | + | |
| 4461 | + | |
| 4462 | + | |
| 4463 | + | |
| 4464 | + | |
| 4465 | + | |
| 4466 | + | |
| 4467 | + | |
| 4468 | + | |
| 4469 | + | |
| 4470 | + | |
4401 | 4471 | | |
4402 | 4472 | | |
4403 | 4473 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1401 | 1401 | | |
1402 | 1402 | | |
1403 | 1403 | | |
1404 | | - | |
| 1404 | + | |
| 1405 | + | |
| 1406 | + | |
| 1407 | + | |
| 1408 | + | |
| 1409 | + | |
| 1410 | + | |
| 1411 | + | |
1405 | 1412 | | |
1406 | 1413 | | |
1407 | 1414 | | |
| |||
Lines changed: 43 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
| 52 | + | |
51 | 53 | | |
52 | 54 | | |
53 | 55 | | |
| |||
83 | 85 | | |
84 | 86 | | |
85 | 87 | | |
86 | | - | |
| 88 | + | |
87 | 89 | | |
88 | | - | |
| 90 | + | |
89 | 91 | | |
90 | 92 | | |
91 | 93 | | |
| |||
96 | 98 | | |
97 | 99 | | |
98 | 100 | | |
99 | | - | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
100 | 106 | | |
101 | 107 | | |
102 | 108 | | |
| |||
110 | 116 | | |
111 | 117 | | |
112 | 118 | | |
113 | | - | |
| 119 | + | |
| 120 | + | |
114 | 121 | | |
115 | 122 | | |
116 | 123 | | |
| |||
119 | 126 | | |
120 | 127 | | |
121 | 128 | | |
122 | | - | |
| 129 | + | |
123 | 130 | | |
124 | 131 | | |
125 | 132 | | |
| |||
137 | 144 | | |
138 | 145 | | |
139 | 146 | | |
140 | | - | |
141 | | - | |
| 147 | + | |
| 148 | + | |
142 | 149 | | |
143 | 150 | | |
144 | 151 | | |
| |||
163 | 170 | | |
164 | 171 | | |
165 | 172 | | |
166 | | - | |
167 | | - | |
| 173 | + | |
| 174 | + | |
168 | 175 | | |
169 | 176 | | |
170 | 177 | | |
| |||
213 | 220 | | |
214 | 221 | | |
215 | 222 | | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
216 | 250 | | |
217 | 251 | | |
218 | 252 | | |
| |||
0 commit comments