Repository navigation
fix(spec)!: refuse a blank string in a flow node's predicate slot — decision branch expression, screen field visibleWhen (#17493) - #19960
Conversation
…hema.parse and in predicateSlotRefusal Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
…egistered) Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 32c93b68e703145a1ce1cda39fb503a114b5c4d6 && git checkout 32c93b68e703145a1ce1cda39fb503a114b5c4d6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3b5607019f6b1f84b14716c9c5e3359a986de08e 58a65d12819cc4bc3a690a9207e3074e805303c7 && git checkout -B drift-repro 3b5607019f6b1f84b14716c9c5e3359a986de08e && git merge --no-ff 58a65d12819cc4bc3a690a9207e3074e805303c7
node scripts/docs-audit/affected-docs.mjs --json 3b5607019f6b1f84b14716c9c5e3359a986de08e
|
Contract reviewServed-tier: Reviewed and posted 2026-09-24T09:53Z by the at-tier review subagent the ① Derived judgmentsClosure — every door a flow reaches the engine through, and what the blank meets there (all measured on the built head unless marked read).
Regression. Census (own numbers). Static, tight regex over every tracked file: 0 blank Stored flows at boot. True as measured for a Pins re-measured (one anchor per ablation, one hunk proved, spec dist rebuilt for A1–A3, restored, tree clean after each). A1 parse call: spec 8 red / sa 7 red / lint green. A2 blank arm: 9 / 8 / 8. A3 resolver: 9 / 9 / 8. A4 lint call site (whole call disabled): lint 11 red = the 8 blank pins + 3 #15572 envelope pins. A5 engine call site (whole call disabled): sa 5 red, all five the #15572 envelope pins; every blank pin stays green — the engine pass is the second line for the blank and the only line for the envelope, exactly what the engine comment and the "Unchanged" paragraph claim. Head re-run after restore: 41 / 34 / 319 green. Uncovered by pins: the The seat's departure (Q2) — measured, and it is NOT behaviour-preserving. Real decision executor (
Other shipped sentences. Verified: the parse-door ② Semver level
③ Boundary flagsBlocking:
Non-blocking:
CI at this head, 46 runs → 35 names after de-dup by latest Implemented-by: VERDICT: FAIL Generated by Claude Code |
…s routing, scope the stored-flow and route sentences Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
…xt does Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1 Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Reviewed and posted 2026-09-24T11:52Z by the at-tier review subagent the ① Derived judgmentsRound-1 items. B1 fixed (below, measured). N1: Closure — every door, what the blank meets there (measured on the built head unless marked read).
Regression. Census (own, static). Pins re-measured (head: spec 41 / sa 38 / lint 319 green; spec-side ablations each with a real spec rebuild, a dist preflight proving the ablated behaviour, restore to 0 diff lines, rebuild, preflight restored):
The seat's departure (Q2) — N5. Built CLI Other shipped sentences. Verified: the entry is verbatim in the regenerated registry (whitespace-normalised, id once); " ② Semver level
③ Boundary flagsBlocking: none. Non-blocking:
CI at this head, 46 runs → 35 names after de-dup by latest Implemented-by: VERDICT: PASS Generated by Claude Code |
…ull — at all three doors (objectstack-ai#19961) (objectstack-ai#20315) Fixes objectstack-ai#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 (objectstack-ai#17493 / PR objectstack-ai#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, objectstack-ai#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 objectstack-ai#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. objectstack-ai#20168's PR objectstack-ai#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 objectstack-ai#19960's decision-branch prescription is "删掉这个分支" (delete the branch). The landed objectstack-ai#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 (objectstack-ai#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, objectstack-ai#20279 included) - Tests (`os-verify-lock`, spec rebuilt on this head): - spec `src/automation` + `src/migrations`: 1005/1005, including objectstack-ai#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 objectstack-ai#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 objectstack-ai#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>
Fixes #17493
Clause-②: no (narrowing)
Executes ruling A (
5651023407). A string that is blank after trimming is refused in two flow-node predicate slots:decisionconfig.conditions[].expressionandscreenconfig.fields[].visibleWhen. The refusal fires atFlowSchema.parse,AutomationEngine.registerFlowandobjectstack validate, with a message led byPREDICATE_SLOT_STRING_REFUSAL.3b5607019f, the dev found no flow in the tree or in the example stacks carrying either blank. The method is in report5811268231.flow.zod.ts: aFlowSchemarefinement over the ledger predicate slots.flow-node-expression-paths.ts: the resolver emits a blank predicate string, andpredicateSlotRefusalrefuses it.engine.tsandvalidate-expressions.ts: comments only.structuralConditionRefusaldocblock now records that ruling A answered its open question, and its doors line is corrected for the node slot.flow-predicate-slot-blank-string-refused;registry.tsis regenerated.5811268231).predicate-slot-blank.test.tsalso 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 (report5812924875).5811310916, corrected by5811904464and5812959979): the parse door stays althoughconfig.conditionhas none. On a decision branch, the prescription that keeps the run isexpression: '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-automationand@objectstack/lintatminor, plus BREAKING (the launch window refusesmajor), with an ADR-0087registeredmarker.🤖 Generated with Claude Code
https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1