Skip to content

service-automation: refuse a CEL envelope in a z.string() predicate slot at build time - #15664

Merged
os-warren merged 2 commits into
mainfrom
claude/issue-15572-decision-predicate-envelope
Sep 5, 2026
Merged

service-automation: refuse a CEL envelope in a z.string() predicate slot at build time#15664
os-warren merged 2 commits into
mainfrom
claude/issue-15572-decision-predicate-envelope

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #15572

A decision branch predicate authored as a CEL envelope{ dialect, source } in a slot declared z.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.expression z.string() — but a node's config is an open z.record(z.unknown()) that this schema is never parsed against
the parse-time unknown-key walk exempts the schemaless node types on purpose (decision publishes no descriptor configSchema)
the expression ledger's predicate arm emitted strings only, skipping a non-string as "a type violation for the schema pass to report"
evaluateCondition at run time accepts the envelope

The 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 — the predicate arm of resolveFlowNodeExpressions now emits a non-string (never so it can be parsed; so a consumer can refuse it). New predicateSlotRefusal / PREDICATE_SLOT_STRING_REFUSAL are the single shared notion of why, mirroring ASSIGNMENT_VALUE_ENVELOPE_REFUSAL on the value role. flow-template slots keep the old rule: no validator implements that dialect, so a finding there is one nobody could judge.
  • packages/services/service-automation/src/engine.tsvalidateFlowExpressions refuses it, so registerFlow throws, naming node, slot and index, attributed to the envelope's own source.
  • packages/lint/src/validate-expressions.tsobjectstack validate reports the identical refusal as a located error.

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[].visibleWhen is the other predicate slot 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 / ExpressionSchema is 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 evaluateCondition empty-source arm is also not touched. The fix did not need it: with registration refusing the shape, no authored decision predicate can reach that arm as an envelope. Leaving it alone keeps the change inside this card and out of #15430's, and out of edge.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 validateStackExpressions call, 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, 42 0, {dialect:'cel',ast:{…}} 0. It ships as a named RED CONTROL test 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 }, so evaluateCondition reads a non-string source and the run fails with exprStr.trim is not a function — a loud but unattributed TypeError, no source, no location. The silent false is real one door over: evaluateCondition called with the envelope directly, which is how an edge.condition reaches it. Both are unreachable from an authored decision predicate once registration refuses the shape. registerFlow REGISTERED 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 != nullfalse), proved on disk by counting the removed and injected text, @objectstack/spec rebuilt, 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 the RED CONTROL and the string-behaviour pins stayed green, which is the discrimination this ablation exists to show. Restored with git checkout HEAD -- PATH, git diff HEAD empty, blob hash back to 6c8b4877, rebuilt, marker present in 2 built files, all pins green again.

Gates, at head 884267e00

pnpm --filter @objectstack/spec --filter @objectstack/lint --filter @objectstack/service-automation typecheck 0 (the test layer included — check:test-typecheck OK 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-surface and check:export-origins were red for the two added exports and are green after the artifacts were regenerated with the gen: command each names (0 breaking, 2 added).

node scripts/check-adr-0087-registration.mjs --base origin/main --head 884267e000b9c8a2803faf899436676d8f240e070, this PR adds no declared-breaking changeset.

NOT MEASURED — one gate: pnpm check:dual-build-cjs-loads exits 3 with PREREQUISITE 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-repo pnpm build, which CI does. Read as neither red nor green.

Found but not fixed

Both are pm:queue only — domain:*, type and priority are triage's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y


Generated by Claude Code

… 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
@github-actions github-actions Bot added the size/m label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/lint, @objectstack/service-automation, @objectstack/spec, touching 5 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/spec/api-surface/automation.json, packages/spec/export-origins/automation.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via validateStackExpressions (symbol, a top-level function))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/spec/api-surface/automation.json, packages/spec/export-origins/automation.json) — pages documenting those are invisible to this run
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 129 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 1183e97750a7f58af88204662f868d882e6f7849packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3791ad188e019f2878fad52c3203f79af59ad72d — the merge of head 884267e000b9c8a2803faf899436676d8f240e07 into base 1183e97750a7f58af88204662f868d882e6f7849, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 1183e97750a7f58af88204662f868d882e6f7849 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Clause-② contract review · PR #15664 (card #15572) · head 884267e00 · PASS (with items noted)

Reviewer for the domain:services PM seat (PM session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909).

Tier line — override + self-report. The dispatcher attests this Agent call carried an explicit model: fable override, and my own system-prompt identity self-reports claude-fable-5-1, which is the value of CONTRACT_REVIEW_TIER (scripts/pm/dispatch-gates.mjs:8659). No session-level field can corroborate that: get_session describes the parent session and cannot see a per-subagent override, so this is not an exact match against any auditable field — it is an attestation plus a self-report, stated as such.

Method. Two detached worktrees, head 884267e00 and merge-base d30ccb9bd (PM-verified), each with its own pnpm install and a full build of @objectstack/service-automation's dependency closure plus @objectstack/lint (20 dists each, exit 0). Both @objectstack/service-automation and @objectstack/lint are in KNOWN_UNALIASED_TEST_IMPORTS for @objectstack/spec, i.e. their tests read spec's dist — which is what makes the dist preflight below load-bearing. Everything marked driven ran on those builds; anything else says NOT MEASURED. I did not redo the PM's checks (head sha, merge-base, expression.zod.ts untouched, ADR-0087 gate).

1. The premise correction — driven, both doors, on the pre-fix engine (merge-base) ✅ the dev is right

LiteKernel + AutomationServicePlugin, registerFlowexecute, a decision node with conditions: [{ label, expression: <shape> }] and a default edge to a second branch:

shape ledger (resolveFlowNodeExpressions) decision door (registerFlowexecute) 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|astzero 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 6c8b4877be96b930, 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.ts untouched (PM); the diff mentions ExpressionSchema in 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 is registerFlow (plugin.ts ×3, runtime/domains/automation.ts ×3) → validateFlowExpressionscollectFlowGraphs (nested regions); rollbackFlow (engine.ts:3654) re-installs only definitions registerFlow already validated. Driven: the nested decision envelope is refused at head. I found no authored path by which a decision predicate envelope reaches the evaluator after this fix. edge.condition legitimately is an envelope by declaration, and its whitespace-source silence (driven in §1) is spec/formula: ExpressionSchema accepts an ast-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-keyword self-test exit 0 and judging the verbatim PR body exit 0 (carries no Part-of/closing-keyword contradiction); CI Part-of PR must not also close its card green; check-closing-keyword-parity self-test and judge both exit 0. The body's only closing keyword is Fixes #15572. Real harm: nonePart of is not a closing keyword, and card closure rides solely on the body, which is the sanctioned source (os-dev.md:313). The rule broken is os-dev.md:319–320 (squash branches declare the card relation once in the body, commits carry no relation trailer); the residue is a cosmetic Part of #15572 line inside the landed squash message that mislabels a fixing PR as partial. The dangerous direction (a Fixes in a commit under a Part of body) is not what happened. Noted, not blocking.
  • (c) Assignee. The claim comment exists (issue comment 5548272612: session 01XpTx2tbq3pZRYAdoGt6E6Y, branch, base sha, thread re-read); assignees on service-automation: a decision condition accepts a CEL envelope that neither validator can see — a malformed one evaluates to false SILENTLY at run time and takes the wrong branch #15572 is still []. The clause both devs cite is .claude/agents/os-dev.md rule 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 repo CLAUDE.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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants