service-automation: refuse a CEL envelope in a z.string() predicate slot at build time - #15664
Conversation
… slot at build time (#15572) A `decision` branch predicate (or a screen field's `visibleWhen`) authored as the `{ dialect, source }` expression envelope reached a shape neither validator could see: the node's `config` is an open `z.record` no Zod schema is parsed against, `validateNodeConfigKeys` exempts the schemaless node types on purpose, and the expression ledger's `predicate` arm skipped every non-string as "a type violation for the schema pass to report" — for `decision`, a schema pass that does not exist. The evaluator accepted it regardless, so the only layer that ever read the predicate was the last one. Fixed at the producer, per the contract-first rule: - `resolveFlowNodeExpressions` emits a non-string in a `predicate` slot so a consumer can refuse it; `predicateSlotRefusal` / `PREDICATE_SLOT_STRING_REFUSAL` are the single shared notion of why. - `registerFlow` throws, located and attributed; `objectstack validate` reports the same refusal as an `error`. String behaviour is untouched: a whitespace-only string still means "not authored" on both sides, and what a non-empty string says is still `validateExpression`'s verdict. `flow-template` slots keep the old rule — no validator implements that dialect. `evaluateCondition`'s empty-source arm is deliberately NOT touched: that is the shared evaluator, and the whitespace-source envelope it turns on is #15430's surface. Part of #15572 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…o new automation exports `predicateSlotRefusal` / `PREDICATE_SLOT_STRING_REFUSAL` are additive — 0 breaking, 2 added — and both artifacts are generated products, not hand-edited ledgers: `check:api-surface` and `check:export-origins` name the exact `gen:*` command that writes them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
📓 Docs Drift CheckThis PR changes 3 package(s): ⛔ 1 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 129 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 3791ad188e019f2878fad52c3203f79af59ad72d && git checkout 3791ad188e019f2878fad52c3203f79af59ad72d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1183e97750a7f58af88204662f868d882e6f7849 884267e000b9c8a2803faf899436676d8f240e07 && git checkout -B drift-repro 1183e97750a7f58af88204662f868d882e6f7849 && git merge --no-ff 884267e000b9c8a2803faf899436676d8f240e07
node scripts/docs-audit/affected-docs.mjs --json 1183e97750a7f58af88204662f868d882e6f7849
|
Clause-② contract review · PR #15664 (card #15572) · head
|
| shape | ledger (resolveFlowNodeExpressions) |
decision door (registerFlow → execute) |
evaluateCondition(shape) direct |
edge door (edge.condition = shape) |
|---|---|---|---|---|
{dialect:'cel', source:' '} |
[] |
REGISTERED → success=false, error="exprStr.trim is not a function" |
false, silently |
REGISTERED → success=true, no error, run summary yes_node: skipped, no_node: success |
{dialect:'cel', source:'rows.map(r,'} |
[] |
REGISTERED → same TypeError | throws Unexpected token: EOF |
refused at registration (parsed) |
{dialect:'cel', ast:{…}} |
[] |
REGISTERED → same TypeError | false, silently |
REGISTERED → success=true, no error |
42 |
[] |
REGISTERED → same TypeError | false, silently |
n/a (edge schema is typed) |
Also driven directly: evaluateCondition({dialect:'cel', source: <envelope>}, …) — the exact wrap the decision executor applies at builtin/logic-nodes.ts:73 — throws exprStr.trim is not a function for all three envelopes. So at the decision door a malformed envelope does not take the other branch silently; the run fails loud and unattributed (no source, no location), exactly as the PR says. The silent false is real one door over: evaluateCondition called with the envelope directly, and through authored metadata that is edge.condition (declared ExpressionInputSchema, flow.zod.ts:526) — where a whitespace-source envelope produces a run summary byte-identical to the string "false" (conditional edge skipped, default branch taken, success=true). That is #15430's whitespace-source spelling, not this card's.
⇒ Card #15572's "serious half" — silent false, wrong branch, at the decision door — was measured at the wrong door, and the p1 rationale in the triage comment (「沉默地走错分支」) rests on it. The dev corrected it rather than shipping the easy story. This matters beyond the PR: triage may want to re-read #15572's priority with the silent-branch behaviour attributed to edge.condition + whitespace source (#15430). Not this PR's job to decide.
At head, all four shapes are refused at registerFlow with the located message; the same envelope nested inside a loop body region is REGISTERED at the merge-base and refused at head (validateFlowExpressions walks collectFlowGraphs, #4347).
2. Class closure — same producer rule, not widening ✅
The ledger carries exactly two predicate-role entries: screen.fields[].visibleWhen and decision.conditions[].expression; flow-template (loop/map collection) and value (assignment.assignments.*) are untouched by the new branch. Both predicate slots are declared z.string() (builtin-node-config.zod.ts:424, schemaless-node-config.zod.ts:381) — one declaration, one reason. Regression sweep for anything this now refuses: repo-wide grep (ts/tsx/js/mjs/json/yaml/md/mdx; excluding tests, dist, node_modules, json-schema) for expression/visibleWhen followed by { dialect|source|ast → zero hits on a flow surface. The only authored visibleWhen envelopes in the repo (packages/metadata/src/__fixtures__/hotcrm-17.1-built-bare-root-predicates.artifact.json) are UI form fields (form.sections[].fields[].visibleWhen, ExpressionSchema-typed) — a different surface from the flow screen node, unaffected. The three example apps with screen nodes carry no visibleWhen envelope. CI Dogfood Regression Gate ×3 green at this head.
3. RED CONTROL ✅ goes red pre-fix, cannot be the answer, ships named
Driven through validateStackExpressions on the pre-fix lint, same slot, same call, same run: brace-trap string 1 issue (invalid CEL predicate: Expected COLON, got RBRACE); {dialect,source:' '} 0; {dialect,source:'rows.map(r,'} 0; {dialect,ast} 0; 42 0; ['a > 1'] 0; whitespace string 0; plain string 0. At head: control still 1 (unchanged), every non-string 1 error starting with PREDICATE_SLOT_STRING_REFUSAL, source attributed (rows.map(r,; '' for ast/number). The control is a string, so it cannot be the answer to an envelope question. Ships as it('RED CONTROL — the brace-trap string on this slot still reports') at packages/lint/src/validate-expressions.test.ts:2141.
4. Ablation re-run — reproduces, dist discipline held ✅
In the head worktree: } else if (entry.role === 'predicate' && value != null) { → } else if (false) {. On disk: source marker 1 → 0, injected text 0 → 1, blob 6c8b4877 → be96b930, one file changed. @objectstack/spec rebuilt (exit 0). Preflight, --absent, marker in the dist's own spelling entry.role === "predicate" && value != null: ✓ dist/: marker absent from all 217 built files — the same 217. Against that build: spec 1 red (the new emit test), service-automation 6 red (5 in decision-predicate-envelope.test.ts + 1 in config-expression-ledger.test.ts), lint 3 red (the three envelope tests) — and the RED CONTROL, the brace-in-CEL string pin, both "leaves string predicates alone" pins and the plain-string run pin stayed green. Restored with git checkout HEAD -- <path>: git diff HEAD empty, blob back to 6c8b4877, rebuilt, preflight present-mode exit 0 (marker present in 2 built files, working tree clean against HEAD), pins 19 / 21 / 272 green, final tree 0 dirty paths. Presence was also established before mutating (2 built files, clean tree), so the absent leg was not vacuous.
One honest wrinkle in my run, not the dev's: my --absent invocation exited 1 on the script's tree half — it derives mutate-vs-restore by looking for the marker in the source diff, and because I passed the dist's double-quote spelling it could not see the source path losing the single-quote spelling, derived "restore leg", and reported the deliberately-mutated file as unrestored. The dist half (the one the ablation depends on) was ✓, and the reds on two dist-resolved consumers corroborate it. Procedure note for the next reader: use a marker spelled identically in src and dist (e.g. && value != null, unique in spec src).
5. Both fences ✅
packages/spec/src/shared/expression.zod.tsuntouched (PM); the diff mentionsExpressionSchemain one test comment only, and nothing else re-cuts it.evaluateCondition's empty-source arm untouched — the engine hunk is the registration pass only. The argument tested: every route into the registry isregisterFlow(plugin.ts×3,runtime/domains/automation.ts×3) →validateFlowExpressions→collectFlowGraphs(nested regions);rollbackFlow(engine.ts:3654) re-installs only definitionsregisterFlowalready validated. Driven: the nested decision envelope is refused at head. I found no authored path by which adecisionpredicate envelope reaches the evaluator after this fix.edge.conditionlegitimately is an envelope by declaration, and its whitespace-source silence (driven in §1) is spec/formula:ExpressionSchemaaccepts anast-only envelope that no engine can evaluate — it validates, it registers, it faults at run time #15430's — correctly left alone.
6. Regenerated baselines ✅
At head, spec dist fresh, tree clean beforehand: pnpm gen:api-surface exit 0, pnpm gen:export-origins exit 0, git diff empty afterwards. Genuine generator output, not hand-edited.
7. String behaviour untouched ✅
Driven on both trees through the decision door: ' ', '', absent, 'true', 'false' all REGISTER and run success=true, identically at base and head; ledger [] for the whitespace string on both; lint 0 on both; evaluateCondition(' ') → false on both.
8. Honesty audit
- (a)
pnpm check:dual-build-cjs-loads→ exit 3 here too:PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. … ⛔ This is NOT a pass: nothing was measured(script lines 1126–1129). Honest, and the dev counted it as neither red nor green. NOT MEASURED by me either (no whole-repo build). - (b) Trailer.
check-partof-closing-keywordself-test exit 0 and judging the verbatim PR body exit 0 (carries no Part-of/closing-keyword contradiction); CIPart-of PR must not also close its cardgreen;check-closing-keyword-parityself-test and judge both exit 0. The body's only closing keyword isFixes #15572. Real harm: none —Part ofis not a closing keyword, and card closure rides solely on the body, which is the sanctioned source (os-dev.md:313). The rule broken isos-dev.md:319–320(squash branches declare the card relation once in the body, commits carry no relation trailer); the residue is a cosmeticPart of #15572line inside the landed squash message that mislabels a fixing PR as partial. The dangerous direction (aFixesin a commit under aPart ofbody) is not what happened. Noted, not blocking. - (c) Assignee. The claim comment exists (issue comment 5548272612: session
01XpTx2tbq3pZRYAdoGt6E6Y, branch, base sha, thread re-read);assigneeson 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 is still[]. The clause both devs cite is.claude/agents/os-dev.mdrule 2 — 「assignee 归 PM:派发原子对已把它设好 … 你恒不写 assignee,到手时它为空也一样(那是 PM 侧半状态,报进summary,照常开工)」 — restated at.claude/skills/pm-dispatch/SKILL.md:524「assignee 字段归 PM:原子对 step 1 设,dev 席恒不写它(os-dev 规则 2 同句)」. Both devs followed the agent definition literally, and it contradicts the repoCLAUDE.md("assign yourself and comment"). The half-state is the PM-side atomic pair not having set the assignee at dispatch. PM's to rule (reconcile the wording, or set it in the atomic pair); not a blocker.
9. #15662 / #15663 ✅
Both exist, both labelled pm:queue only — no domain:*, no type, no priority, no assignee. #15662 describes evaluateCondition answering a silent false for a non-string predicate (42, true, ['a']) and a non-string config.condition registering clean — the structural arm this fix does not reach; #15663 describes validateExpression throwing a raw TypeError: source.trim is not a function at packages/formula/src/validate.ts:561, crashing registerFlow unlocated. Both match what the PR says.
CI at this head (read 2026-09-05T02:36Z)
Build Core, all four Type Check jobs, Lint & Repo Gates, Test Core 2–6, Dogfood Regression Gate ×3 + Dogfood Verify CLI, Temporal Conformance, Check Changeset, Governed Surface Queue Guard, both duplicate-claim guards, Part-of guard: success. Test Core (1/6): still in progress at that read.
NOT MEASURED
- The full 68-gate family end to end (I ran: partof, parity, dual-build,
gen:api-surface,gen:export-origins, and the three packages' changed test files at head and under ablation — not the full suites or local typecheck; CI's Type Check jobs are green at this head). - Whole-repo build; browser/dogfood.
Verdict
PASS. The premise correction is right and driven on both doors; the class closure is one declaration and refuses nothing currently authored; the RED CONTROL goes red pre-fix and ships; the ablation reproduces with the dist proven ablated and the controls green; both fences held; the baselines are generator output; string behaviour is unchanged; the follow-ups are correctly fenced. Noted, none blocking: (i) #15572's p1 rationale was measured at the wrong door — worth a triage re-read; (ii) the cosmetic commit trailer; (iii) the assignee half-state and the os-dev.md / CLAUDE.md contradiction — PM's ruling.
Generated by Claude Code
Fixes #15572
A
decisionbranch predicate authored as a CEL envelope —{ dialect, source }in a slot declaredz.string()— reached a shape that neither validator could see, and was executed anyway. This refuses it at the producer.The hole, restated from the four declarations
DecisionConditionSchema.expressionz.string()— but a node'sconfigis an openz.record(z.unknown())that this schema is never parsed againstdecisionpublishes no descriptorconfigSchema)predicatearmevaluateConditionat run timeThe third row is the load-bearing one: for a schemaless node type there is no schema pass, so the value it deferred to nobody was read by nobody — and the evaluator, the one layer with an opinion, was the last one.
The fix — minimal, and at the producer
packages/spec/src/automation/flow-node-expression-paths.ts— thepredicatearm ofresolveFlowNodeExpressionsnow emits a non-string (never so it can be parsed; so a consumer can refuse it). NewpredicateSlotRefusal/PREDICATE_SLOT_STRING_REFUSALare the single shared notion of why, mirroringASSIGNMENT_VALUE_ENVELOPE_REFUSALon thevaluerole.flow-templateslots keep the old rule: no validator implements that dialect, so a finding there is one nobody could judge.packages/services/service-automation/src/engine.ts—validateFlowExpressionsrefuses it, soregisterFlowthrows, naming node, slot and index, attributed to the envelope's ownsource.packages/lint/src/validate-expressions.ts—objectstack validatereports the identical refusal as a locatederror.One list, two consumers, one refusal: the reject set of registration and the reject set of author time are one set, derived once. It closes a class, not one node type —
screen.fields[].visibleWhenis the otherpredicateslot and is refused on the same rule.String behaviour is untouched, deliberately (the card states this boundary so nobody "fixes" it): a whitespace-only string still means "not authored" on both sides, and what a non-empty string says is still
validateExpression's verdict, brace trap and all. Only the shape moved.Scope fence — answered explicitly
⛔
packages/spec/src/shared/expression.zod.ts/ExpressionSchemais not touched: that is #15430's surface, a different mechanism (a shape both validators see and accept), and triage has not ruled whether the two cards are one. Nothing here decides that for them.⛔ The shared
evaluateConditionempty-source arm is also not touched. The fix did not need it: with registration refusing the shape, no authoreddecisionpredicate can reach that arm as an envelope. Leaving it alone keeps the change inside this card and out of #15430's, and out ofedge.condition, which legitimately is an envelope.Measurements
The lint half was re-established with a control that can go red, as the card required. The filed zeros used a second envelope as their control — a control that could itself have been the answer. The replacement is the brace-trap string on the same slot, through the same
validateStackExpressionscall, in the same run: it cannot be the answer, and it reports. Before the fix: control 1 issue,{dialect:'cel',source:' '}0,{dialect:'cel',source:'rows.map(r,'}0,420,{dialect:'cel',ast:{…}}0. It ships as a namedRED CONTROLtest beside the pins, so the next reader inherits it.A premise correction, driven end to end. Through the decision door the malformed envelope does not take the other branch silently. The executor wraps the authored value as
{ dialect: 'cel', source: THE_ENVELOPE }, soevaluateConditionreads a non-stringsourceand the run fails withexprStr.trim is not a function— a loud but unattributed TypeError, no source, no location. The silentfalseis real one door over:evaluateConditioncalled with the envelope directly, which is how anedge.conditionreaches it. Both are unreachable from an authoreddecisionpredicate once registration refuses the shape.registerFlowREGISTERED and the ledger[]readings are confirmed exactly as filed.Every new pin was mutated to confirm it discriminates. The fix was committed first, then the resolver's new emit branch was ablated (
entry.role === 'predicate' && value != null→false), proved on disk by counting the removed and injected text,@objectstack/specrebuilt, and the dist proved to no longer carry the marker (ablation-dist-preflight … --absent: marker absent from all 217 built files). Against that build: spec 1 red, service-automation 6 red, lint 3 red — and theRED CONTROLand the string-behaviour pins stayed green, which is the discrimination this ablation exists to show. Restored withgit checkout HEAD -- PATH,git diff HEADempty, blob hash back to6c8b4877, rebuilt, marker present in 2 built files, all pins green again.Gates, at head
884267e00pnpm --filter @objectstack/spec --filter @objectstack/lint --filter @objectstack/service-automation typecheck0 (the test layer included —check:test-typecheckOK on both packages that carry one). Full suites: spec 472 files / 12690 tests, lint 95 / 2983, service-automation 108 / 1299, all 0.The gate family was re-derived from the real changed set with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(8 paths, merge base d30ccb9). All 68 derived gates run, exit code captured by redirect, never through a pipe: 67 exit 0.check:api-surfaceandcheck:export-originswere red for the two added exports and are green after the artifacts were regenerated with thegen:command each names (0 breaking, 2 added).node scripts/check-adr-0087-registration.mjs --base origin/main --head 884267e000b9c8a2803faf899436676d8f240e07→ 0, this PR adds no declared-breaking changeset.NOT MEASURED — one gate:
pnpm check:dual-build-cjs-loadsexits 3 withPREREQUISITE NOT MET — this gate reads built output, and some package has no dist(49 packages unbuilt in this worktree; its own words: "⛔ This is NOT a pass: nothing was measured"). It needs a whole-repopnpm build, which CI does. Read as neither red nor green.Found but not fixed
evaluateConditionanswers a silentfalsefor a non-string predicate, and a non-stringconfig.conditionregisters clean #15662 —evaluateConditionanswers a silentfalsefor a non-string predicate (42,true,['a']all measured), and a non-stringconfig.conditionregisters clean and runs. The structural condition surface, deliberately not a ledger entry, so this fix does not reach it.validateExpressionthrows a rawTypeError: source.trim is not a function, so a bad condition crashesregisterFlowwith an internal message instead of a located refusal #15663 —validateExpressionthrows a rawTypeError: source.trim is not a function(packages/formula/src/validate.ts:561), so a bad condition crashesregisterFlowwith an internal message instead of the located refusal that function's callers are built around.Both are
pm:queueonly —domain:*, type and priority are triage's.🤖 Generated with Claude Code
https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Generated by Claude Code