fix(spec)!: refuse a decision branch with no expression — absent or null — at all three doors (#19961) - #20315
Conversation
…ate-slot walk (#19961) The expression ledger marks decision.conditions[].expression required, so resolveFlowNodeExpressions emits an absent or null value there and every door refuses it through predicateSlotRefusal, under the blank's lead sentence. FlowSchema.parse admits the absent value of a required slot into its refinement; the lint pass drops its null early return so the resolver decides what is a finding. Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude <noreply@anthropic.com>
…edicate at all three doors; ADR-0087 entry; changeset (#19961) Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude <noreply@anthropic.com>
…; reconcile required over the predicate role (#19961) Claude-Session: https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN Co-authored-by: Claude <noreply@anthropic.com>
…cision-branch-expression-absent
…cision-branch-expression-absent
…cision-branch-expression-absent
📓 Docs Drift CheckThis PR changes 2 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 5 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 145a1c2726afdb34baf6ac336b676ae944843056 && git checkout 145a1c2726afdb34baf6ac336b676ae944843056
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7b1e4a48714ded619ca58c4645a398f284c30dd3 7534fd7ea1aa0c3f49347eff689227575dc0d1a8 && git checkout -B drift-repro 7b1e4a48714ded619ca58c4645a398f284c30dd3 && git merge --no-ff 7534fd7ea1aa0c3f49347eff689227575dc0d1a8
node scripts/docs-audit/affected-docs.mjs --json 7b1e4a48714ded619ca58c4645a398f284c30dd3
|
Contract reviewServed-tier: ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #19961
Clause-②: no (narrowing)
A
decisionbranch with noexpression(the key absent, orexpression: null) is now refused at all three doors:FlowSchema.parse,AutomationEngine.registerFlowandobjectstack 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/maina9fb83efDecisionConditionSchemadeclares a branch{ label, expression }withexpressiona requiredz.string(). Nothing parses a decision node's openconfigagainst 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.FlowSchema.parseregisterFlowobjectstack validate --json{ label: 'y' }(the card's shape)valid: true, exit 0{ label: 'y', expression: null }valid: true, exit 0{ label: 'y', condition: 'true' }(the edge's spelling)valid: true, exit 0{ label: 'y', expression: ' ' }(control, #19960)customatnodes.1.config.conditions.0.expressionvalid: false, exit 1, same path{ label: 'y', expression: 'true' }(control)valid: true, exit 0What the run did with it:
evaluateCondition({ dialect: 'cel', source: undefined })andsource: nullboth throwcondition evaluation error: A structural condition …. On the real decision executor, a run that reaches such a branch endssuccess: falseat the branch (pinned below).The fix: one walk, one judge
packages/spec/src/automation/flow-node-expression-paths.tsFlowNodeExpressionPathgainsrequired?: true. It is set ondecisionconditions[].expressionand on nothing else.requiredpredicate slot,resolveFlowNodeExpressionsnow emits the absent ornullvalue on a branch that exists. It still skips a decision with noconditions, an empty list, and an absent screenvisibleWhen.predicateSlotRefusal(undefined | null)now has its own detail sentence and prescription, under the unchangedPREDICATE_SLOT_STRING_REFUSALlead.packages/spec/src/automation/flow.zod.ts: theFlowSchemapredicate-slot refinement now admits the absent ornullvalue of arequiredslot, next to strings. Every other non-string keeps its service-automation: adecisioncondition accepts a CEL envelope that neither validator can see — a malformed one evaluates tofalseSILENTLY at run time and takes the wrong branch #15572 scope, so it is still not refused at this door.packages/lint/src/validate-expressions.ts:checkDeclaredPredicatedropped itsraw == nullearly return. Whether an absent value is a finding is the resolver's call. The early return answered "valid" for the exact valueFlowSchema.parserefuses, for any caller ofvalidateStackExpressionsthat 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 callspredicateSlotRefusalon everything the resolver emits, andregisterFlowparses 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
expressionas a refused slot. I did not choose (b), parsing each branch againstDecisionConditionSchema. Reasons, per axis:rowsToList, which writes{ label }. The absent key is the whole defect.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.conditionalias mistake in the same prescription.DecisionConditionSchemaand the fencedDecisionConfigSchema/moderegion are untouched. #20168's PR #20279 landed while this was in flight, and this branch is merged over it (7534fd7e). Its refusal lives inDecisionConfigSchema, which no door parses a node's config against, so it and this walk do not meet. Its suite is green here (in thesrc/automationrun below).Prescription wording. Triage (⚠️ not by dropping a decision's only branch. That clause is pinned by
5811370954) says PR #19960's decision-branch prescription is "删掉这个分支" (delete the branch). The landed #19960 text says something else: write the predicate, orexpression: 'false'to keep what the blank ran, andpredicate-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)For
null,Found nothing — the key is absentreadsFoundfollowed by the code-spellednull. 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: nullandcondition: 'true'all giveobjectstack validate --jsonvalid: false, exit 1, onecustomerror atflows.0.nodes.1.config.conditions.0.expression. The absent and alias messages are byte-identical.registerFlowrefuses the same three with acustomissue atnodes.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, thepath, and the full message equal to the spec's ownpredicateSlotRefusal(value).message.packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts:FlowSchema.parse.null,conditionalias, blank control, real accept control.conditionsor[]still parses; an absent screenvisibleWhenstill parses.packages/services/service-automation/src/decision-branch-expression-absent.test.ts:registerFlow.getFlowisnullafter each refusal.success: false,condition evaluation error, ran['start']);expression: 'false'routes to the fallback.packages/lint/src/validate-expressions.test.tsdescribe('a decision branch with no expression (#19961)'):validateStackExpressions, with the same table, the exactwherestring, branch index 1, and controls.packages/spec/src/automation/flow-node-expression-paths.test.ts:undefinedornullfor the decision slot and skips everything else.predicateSlotRefusal(undefined | null)prescription clauses are pinned by name.requiredset is pinned to exactlydecision.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-Schemarequiredlist. It asserts that the ledger'srequiredflags equal the channel's, in both directions, over thepredicaterole. It derives, not assumes, thatvisibleWhenis optional. It asserts thatrequiredis never set on another role. The channels do requireloop.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.tsasserteddecisionFlow('str_absent', undefined)registers. It was re-judged in place, and the reason is written beside it. It now asserts the throw carriesPREDICATE_SLOT_STRING_REFUSALandFound 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/.yamlfile that mentions bothdecisionandconditionsfound 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 adecisionwithconditions.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 emptygit diff HEAD.requiredflag neutralised.required: true,was replaced by a spread that is{}unless aglobalThisflag namedABLATION_19961is set.ablation-dist-preflight.mjs @objectstack/spec ABLATION_19961found the marker present in 20 built files.nulland alias rows, index 1, region, therequired-set pin, the resolver pin);--absentmarker gone from all 222 built files, whole-treegit statusclean. spec 52/52, service-automation 34/34 and lint 344/344 green.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/**ate702ebd4: 3 flows carrydecisionnodes (app-crmconvert-lead, app-showcaseneeds_exec/triage, app-todocheck_recurring). All of them branch on out-edges and declare noconditions, so 0 branches lack anexpression.packages/**non-test: no default flow carries adecisionnode. Thecontent/docs/automation/flows.mdxexamples: 3conditionslists, all withexpression.origin/main96eb092f: 0decisionnodes.service-ai-studio's authoring whitelist namesdecisionas an authorable node type, so AI-authored flows now meet this refusal.f8a9d0fb(.objectui-sha):FlowObjectListFieldrowsToListstill 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
flow-decision-branch-expression-absent-refused(major 18) and a regeneratedregistry.ts.'false'would change behaviour rather than keep it..changeset/19961-decision-branch-expression-absent-refused.md:@objectstack/specand@objectstack/lintminor, BREAKING, with the FROM → TO table.service-automationgets no changeset: its diff is test files only, and those are not infiles[].Verification (final head
7534fd7e, which isorigin/main6a6a17b6merged, #20279 included)os-verify-lock, spec rebuilt on this head):src/automation+src/migrations: 1005/1005, including fix(spec): refuse decisionmodebeside a non-emptyconditionslist (#20168) #20279'sschemaless-node-config.test.ts.validate-expressions.test.ts: 344/344.266cd043: spec 16855 passed (583 files), service-automation 1767/1767, lint 4269/4269.266cd043: spec, lint and service-automation all exit 0. No file of this diff changed after that.dispatch-gates.mjs --commandsre-derived on7534fd7egives 90 families. 89 ran with exit 0.dispatch-gates --rananswers "90 derived famil(ies) accounted for — 89 run, 1 NOT-MEASURED".check:type-check-debt. Its--re-measureruns a whole-treeturbo run build --filter=./packages/*outsideos-verify-lock. This diff touches no DEBT-ledger package.check:dual-build-cjs-loadsanswered PREREQUISITE NOT MET because 12 unrelated packages were unbuilt.check:dts-closurewent red on local state: 6 packages lost their.d.tsto my own interrupted--re-measurebuild. That is not this diff.eslint --no-inline-config --format jsonover the 12 changed.tsfiles on7534fd7e: 12 files, 0 errors, 0 warnings.eslint.config.mjsfiles: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}'], and no file here is ignored.parserOptions.project), so this diff cannot move a verdict on an untouched file.origin/mainmerges, 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 fix(spec): refuse decisionmodebeside a non-emptyconditionslist (#20168) #20279'sDecisionConfigSchemamode), not the flow predicate walk.Acceptance notes (observed, not filed)
service-automationengine.tsevaluateConditionstill has an inline comment saying the empty-source arm is where "adecisionnode whoseconditions[]entry has noexpression" lands and answersfalse. Since service-automation:evaluateConditionstill throws a rawTypeError: exprStr.trim is not a functionon a non-string envelopesource— registration refuses the shape, evaluation faults unattributed #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 editsevaluateCondition, else none.Generated by Claude Code