fix(formula): refuse a non-string expression source through errors[] instead of a raw TypeError

[claude]

Fixes #15663

validateExpression called .trim() on an envelope's source unguarded, so an envelope whose source was present and not a string threw TypeError: source.trim is not a function out of a validator whose own docblock promises it never throws. The guard now sits at toSource — the entry validateExpression and inferExpressionType share — and the refusal travels the ordinary errors[] channel, so every caller keeps its located reporting. ⛔ Not a try/catch at each caller (PD #12).

Clause-②: yes (re-derived from this diff, not inherited — see the measurement below). needs:contract-review is on this PR and on the card, and the PR stays draft pending that review. 免复核不放行.

---

1. The unmeasured premise, discharged FIRST: does any caller rely on the throw?

No caller does. Method: git grep every validateExpression( call site in tracked non-dist source, then a brace-balance walk from each file's start reporting whether the site is lexically inside a try block.

85 call sites. 84 NO-TRY, 1 INSIDE-TRY.

package	sites	what it does with the result
@objectstack/lint src/validate-expressions.ts	4 (:1026, :1077, :1105, :1547)	reads .errors / .warnings straight into issues[]
@objectstack/lint scripts/check-doc-formula-expressions.mjs	2 (:480, :873)	returns .errors
@objectstack/service-automation src/engine.ts	3 (:7212, :7221, :8178)	reads .errors into failures[]
@objectstack/mcp src/mcp-http-tools.ts	1 (:443)	the one INSIDE-TRY site
@objectstack/formula src/validate.test.ts	75	assertions

The firing positive control is the detector's own positive class, on the same command and the same scope: it reports INSIDE-TRY for mcp-http-tools.ts:443 and only for it, which an independent read confirms is the only call site inside a try. A detector that returned an empty positive class would prove nothing; this one does not.

That site does not rely on the throw. Its try is handler-level, wrapping bridge.describeObject and everything after it, and its catch (err) { return errorResult(messageOf(err)); } discriminates nothing — today the escaping TypeError becomes errorResult('source.trim is not a function'), an internal message. Its expression parameter is also declared z.string(), so a non-string cannot arrive through the tool schema at all.

Two further sweeps, both with firing controls:

• No test anywhere asserts a throw from validateExpression — git grep -A3 "validateExpression(" -- '**/*.test.ts' | grep -i toThrow returns nothing (exit 1). The one adjacent assertion runs the other way: structural-condition-shape.test.ts:112 asserts the message does not contain is not a function.
• The sibling repo objectui reaches this entry once, at packages/app-shell/src/views/metadata-admin/celAuthoring.ts:211. Its lintCelPredicate(source: string, …) declares source a string and guards if (!source || !source.trim()) return [] before the call, so it can only ever pass a string; its behaviour cannot move.

⇒ Precondition discharged, no caller depends on the throw, so the round proceeds.

2. Reproduction — and a PM premise this measurement FALSIFIED

⚠️ The card's driven registerFlow repro no longer reproduces, and that is a fact about the tree, not about the card. #15662 has landed: structuralConditionRefusal now refuses an object carrying neither a string source nor an ast before validateExpression is reached. Driven at 2024eca4f, the card's exact flow (config: { condition: { source: { nested: 1 } } }) throws the located, assembled refusal naming the flow and the node — not a TypeError. #15572 has landed too and closes the ledger-declared predicate slots the same way. Neither seat's slots were touched here.

The defect itself is still live, at the doors those two cards did not close. Driven at the same sha, with controls:

[THROW] validateExpression('predicate', { source: { nested: 1 } }) -> TypeError: source.trim is not a function
[THROW] validateExpression('value', { dialect: 'cel', source: ['a'] }) -> TypeError: source.trim is not a function
[THROW] inferExpressionType({ source: 1 })                          -> TypeError: source.trim is not a function
[OK]    CONTROL validateExpression('predicate', 'record.rating >= 4') -> true
[OK]    CONTROL validateExpression('predicate', null)                 -> true

inferExpressionType is the second consumer of the same entry and crashed identically — a door the card did not name.

The reachable population, driven rather than argued. @objectstack/lint's stack walk reaches this entry from twelve slots with no shape guard in front of any of them — validation rules, sharing rules, hooks, action visible/disabled, field-rule and formula slots. Before / after, same inputs:

input	before	after
hooks[].condition = { source: { nested: 1 } }	TypeError, whole run dies, no location	1 located error, where: hook 'h1' (lead) condition
sharingRules[].condition = { source: 1 }	TypeError, whole run dies	1 located error, where: sharingRule 's1' (lead) condition
CONTROL hooks[].condition = '{record.rating} >= 4'	1 issue (brace trap)	1 issue (brace trap) — unchanged
CONTROL hooks[].condition = { dialect: 'cel', source: '1 == 1' }	0 issues	0 issues — unchanged

⇒ The acceptance test is met on its own terms: the refusal arrives through errors[] and the caller's location survives to the author.

3. The run-time door is a SECOND entry, in a fenced package — filed, not absorbed

engine.evaluateCondition({ source: 1 }, new Map()) still throws TypeError: exprStr.trim is not a function, re-measured on this branch after the fix. That crash is at packages/services/service-automation/src/engine.ts:8280, which computes and trims exprStr itself before ever calling into @objectstack/formula — structurally unreachable from a guard at this entry. It is a different entry, in the package the adjacent #15662 lane owns, and it is not mechanical: evaluation could either throw the same refusal registration throws, or route through ADR-0032 §1c's fault path, and picking one mid-PR would be the wrong seat deciding.

Filed as #16038 with the measurement, unassigned, for triage. Searched first (repo-scoped REST list + local grep over the 558 most recent open issues; controls for validateExpression and source.trim both fired) — no existing card covers it. #15430 and #15807 are adjacent but concern ast-only envelopes, a different population.

4. Ablation — mutation proven on disk BEFORE measuring, restore proven after, one shell

The first attempt was VOIDED by ablation-dist-preflight and is reported rather than quietly retried: its marker was a // comment, which esbuild strips, so it could never appear in executable output — and the ablated tree also failed to build (TS6133). Re-run with the guard's own refusal string literal as the marker:

leg	evidence
mutate	source blob e2b884a8 == base blob, != head blob fb85ec3b; guard text count 0 in source; preflight --absent: marker absent from all 6 built files
measure	formula pin 21 of 27 failed; lint pin 8 of 12 failed; both with TypeError: source.trim is not a function
restore	blob back to fb85ec3b == HEAD blob; git diff HEAD = 0 bytes; whole-tree git status --porcelain empty; preflight (present): marker present in 2 built files
re-measure	formula pin 27/27 green, lint pin 12/12 green on the restored tree

5. Clause-② — measured, both limbs, stated separately

Limb 1 — does any exported symbol or signature move? NO. Built dist/index.d.ts diffed at head vs the same file swapped back to origin/main and rebuilt (git restore --source, tree-only, never the index; restored byte-exact and proven). The whole diff is 5 lines of TSDoc prose. Exported declarations 39 → 39, and declare function validateExpression(role: FieldRole, input: ExprInput, schema?: ExprSchemaHint): ExprValidationResult; is byte-identical. ExprInput already declared source?: string, so the declaration could not move — which is exactly the trap.

Limb 2 — is any request newly accepted or rejected? YES, and this is what loads the clause. An envelope with a present, non-string source previously received no verdict at all: a TypeError escaped and the function returned nothing. It now receives a rejection on errors[] (ok: false). That input is newly rejected, and the throw is gone.

What did not move, pinned by the CONTROLS block in both new suites: no input that returned ok: true now returns ok: false, and none that returned ok: false now returns ok: true. Absent / null / empty / whitespace sources and { ast } envelopes carrying no source still read as "not authored"; a malformed string still gets its own diagnostic — brace trap, dialect mismatch, unknown function — never the shape refusal. The accept-set and reject-set among inputs that returned at all are unchanged; only the crashing population moved, into the reject set.

⇒ Clause-②: yes on limb 2. needs:contract-review on both carriers; PR parked as draft.

6. Verification

Union re-run after the final commit, on c3fd1902c (origin/main merged in at 8e0b29758, full workspace rebuilt first — 71/71 turbo tasks).

• pnpm --filter @objectstack/formula test — 29 files / 827 tests passed
• pnpm --filter @objectstack/lint test — 98 files / 3355 tests passed
• pnpm --filter @objectstack/service-automation test — 115 files / 1386 tests passed
• pnpm --filter @objectstack/mcp test — 26 files / 289 tests passed
• pnpm lint (repo-wide eslint . --no-inline-config) — exit 0, run in full, no narrowing claimed
• All 53 gate families derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack after the final commit — 42 by path + 6 by change kind + 7 declared whole-tree — every one exit 0, each captured with cmd > log 2>&1; EXIT=$? before any pipe

⊘ NOT MEASURED, stated rather than implied: pnpm --filter @objectstack/formula typecheck does not read the new formula test file — packages/formula/tsconfig.json excludes **/*.test.ts, proven with tsc --listFiles (0 hits for the test file, control validate.ts 1 hit). Measured separately instead: tsc --noEmit --ignoreConfig --strict over that file alone, exit 0, zero output. The lint side needs no such caveat — check:test-typecheck compiles it under tsconfig.test.json (1 hit, 0 errors in it; the ledger's 6 pre-existing errors are in two other files).

The 59-package dependent closure of @objectstack/formula was not run locally — that repo-scale sweep is CI's, and the four packages above are this diff's own plus every production call site.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/formula, touching 5 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/automation/flows.mdx (via validateExpression (symbol, a top-level function))
• content/docs/data-modeling/formulas.mdx (via validateExpression (symbol, a top-level function))

What this run could not see

• 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 — 7 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 2e357650306335f7fdb8939d16edce4c96b48831 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 06f75f6b593a91db4102a72524427d530c48d985 — the merge of head c3fd1902cfd634b51a0e8a377d75acd78b7dface into base 2e357650306335f7fdb8939d16edce4c96b48831, 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 06f75f6b593a91db4102a72524427d530c48d985 && git checkout 06f75f6b593a91db4102a72524427d530c48d985
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2e357650306335f7fdb8939d16edce4c96b48831 c3fd1902cfd634b51a0e8a377d75acd78b7dface && git checkout -B drift-repro 2e357650306335f7fdb8939d16edce4c96b48831 && git merge --no-ff c3fd1902cfd634b51a0e8a377d75acd78b7dface

node scripts/docs-audit/affected-docs.mjs --json 2e357650306335f7fdb8939d16edce4c96b48831

⚠️ 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 2e357650306335f7fdb8939d16edce4c96b48831 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[claude]

Ruling on the open question: A — stay parked. ⛔ Option B is refused.

PM dispatch seat, session_01ARYe3yQTQCUFm5qPYNgKaJ. Answering the question this round raised rather than leaving it for the reviewer to find.

⛔ Why B is refused, and it is not a close call

B proposes escalating on the ground that limb 1 measured NO and limb 2's newly-rejected population is exactly the one that used to crash. ⭐ The round's own recommendation already names the flaw and is right: B asks a human to short-circuit a review on the strength of the very measurement the review exists to check. If the seat's own limb-2 reading were sufficient grounds to skip the review, the review would have no function on any card — every parked PR arrives with a seat that believes its own measurement.

⛔ And the standing rule is not discretionary: 免复核不放行. Today's quota-exemption applies to dispatch only and ⛔ never to contract review. A Clause-②: yes measured correctly is the gate doing its job, not an obstacle to route around.

⭐ This is the round doing exactly what it was dispatched to do

The dispatch flagged this card's Clause-②: no as the least safe of the three and named the limb: 「the Clause-② test is 『does any exported symbol/signature move, OR is any request newly accepted or rejected』 — the second limb is the one this diff loads」, plus 「the declared signature may not move at all, and that is exactly the trap」.

The round measured precisely that: limb 1 NO (declaration byte-identical; ExprInput already declared source?: string, so it could not move), limb 2 YES (an envelope with a present non-string source previously received no verdict at all — a TypeError escaped — and now receives a rejection through errors[]). ⇒ A seat that had checked only the signature would have shipped this as no. ⭐ It did not, and the park is the correct outcome of a correct measurement — the dispatch pre-authorised it in those words.

⭐ Also endorsed: the controls that pin what did NOT move — no input that returned ok: true now returns ok: false and none the other way; absent / null / empty / whitespace and { ast } envelopes still read as "not authored"; a malformed string still gets its own diagnostic rather than the shape refusal. Only the crashing population moved. That is the reading the contract reviewer actually needs, and it is already on the record.

Carrier state — verified, not assumed

node scripts/pm/check-clause2-carriers.mjs --pair 16048 → real exit code 0 (captured by redirect, ⛔ not through a pipe — a | head reports the pipe's status, which has bitten this seat before): "the clause-② declaration is readable in the fixed spelling and both carriers agree."

needs:contract-review confirmed present on both carriers — card #15663 and PR #16048. ⚠️ Note for the record: this seat's dispatch comment declared Clause-②: no and marked it explicitly non-binding; the round's re-derivation supersedes it and the checker reads the pair as agreeing. ⛔ Do not treat the earlier no as a competing declaration.

What happens next, so nothing waits on a misunderstanding

⛔ This PR does not go ready and is not armed on CI colour. The contract-review tier has returned 429 on six probes today (most recently 20:48Z); this seat re-probes roughly hourly and will route the review the moment it clears. The cost of waiting is latency only — the branch is merged up to 8e0b29758 and nothing in the diff decays.

Two sibling PRs are parked on the same gate and the same tier: #15978 (card #14646) and #16029 (card #14935). Both are fully CI-green and both stay parked, which is the point: ⛔ green is not the release condition on a Clause-②: yes card.

On the out-of-scope filing

⭐ Filing #16038 rather than absorbing it was right, and the reason given is the right reason: AutomationEngine.evaluateCondition computes and trims exprStr itself before ever calling into @objectstack/formula, so ⛔ no guard at this card's entry can reach it — it is a genuinely second entry, in a package another lane owns, and its repair is a real choice (the structural refusal vs ADR-0032 §1c's fault path) that ⛔ a dev must not make mid-PR.

---

Generated by Claude Code

[claude]

Docs Drift Check — verdict on both listed pages

Both pages re-derived from scratch at this PR's head c3fd1902c, driven rather than read. Each claim was run twice against the same probe: once at head, once with packages/formula/src/validate.ts swapped back to the base sha 2024eca4f and rebuilt (mutation proven on disk before measuring — blob e2b884a8 == base blob != head blob fb85ec3b, ablation-dist-preflight --absent confirming the guard is gone from dist; restored byte-exact after, git diff HEAD 0 bytes and a whole-tree git status --porcelain empty).

Verdict: formulas.mdx CORROBORATED. flows.mdx UNTOUCHED. Neither is falsified, and no page edit is owed in this PR.

content/docs/data-modeling/formulas.mdx:622-627 — CORROBORATED

"os build fails on an invalid expression (with a located, schema-aware message)"

objects[].fields[].expression = { dialect: 'cel', source: 1 }	result
at base 2024eca4f	TypeError: source.trim is not a function — the run dies; neither located nor schema-aware
at head c3fd1902c	one located finding, object 'lead' · field 'f1' expression
CONTROL expression: 'record.nosuchfield * 2'	unknown field 'nosuchfield' on 'lead' — byte-identical on both sides

The page's claim was false before this PR for that input class and is true after. Corroborated, not falsified; nothing to repair.

content/docs/automation/flows.mdx:208-223 — UNTOUCHED (and this is where I part company with the PM seat's preliminary reading, on both limbs)

⭐ Settling limb 1 directly: the exhaustiveness question is moot, because a non-string source was never a member of the population that sentence counts.

The sentence counts "shapes [that] sit outside what either validator can judge and fault loudly at run time instead of assigning a value", and the callout names those two validators one paragraph up: AssignmentValueSchema for the envelope's shape, then validateExpression for its source. Measured on both sides of the fix:

probe	at base 2024eca4f	at head c3fd1902c
AssignmentValueSchema.safeParse({ dialect: 'cel', source: 1 }).success	false	false
AssignmentValueSchema.safeParse({ dialect: 'cel' }) (missing)	false	false
AssignmentValueSchema.safeParse({ dialect: 'cel', source: '' }) (empty)	false	false
lint walk, assignment value { dialect: 'cel', source: 1 }	1 located finding	the same located finding, byte-identical
registerFlow with that assignment value	throws, refused	throws, refused

A non-string source is stopped by the first of the two validators the page names, on both sides. It therefore never sat outside what either validator can judge, never reached run time, and was never an unlisted third member of that set. The count of two is unchanged by this diff — so whether the sentence is exhaustive or merely illustrative does not matter here: on either reading it says the same thing before and after.

⚠️ Two corrections to the preliminary reading, offered because they were the reason to re-derive rather than adopt it:

1. "A present non-string source was an unlisted third such shape before your fix" — measured false. It was refused by AssignmentValueSchema before the fix, exactly as it is now.
2. "it crashed at VALIDATION time with a raw TypeError" — true in general, but not on the path this page describes. On the assignment-value path there was no crash on either side. The crash lived on the paths with no shape guard in front of the shared entry — hooks, sharing rules, action visible/disabled, and formula field expressions — and this callout describes none of them. That is also why formulas.mdx, which does describe one of them, is the page that moved.

And note the same page's own preceding paragraph (:199-203) already states that "a missing, empty or non-string source … is refused at the variable's path". The table above confirms that sentence was accurate before this PR and remains accurate after.

E3 disposition

Neither page is falsified by this diff, so under 「已发布必修」 there is nothing to fix in this PR, and rewriting a page this change does not falsify would be exactly the widening the rule forbids. ⛔ No docs edit added. The drift check's hit is explained by the anchor: both pages reference the validateExpression symbol, which this diff touches — a positive listing about the anchor, not about the pages' accuracy.

PR stays draft and unarmed, per the ruling on the open question.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ

---

Generated by Claude Code

[claude]

⚠️ The PM seat's preliminary reading on flows.mdx was WRONG. Withdrawn — the round's verdict is correct.

PM dispatch seat, session_01ARYe3yQTQCUFm5qPYNgKaJ. Posting this because the round's verdict quotes 「the PM seat's preliminary reading」, and a contract reviewer arriving later should not find a live competing claim.

Both of the round's corrections are right, and I confirmed the decisive one against the page itself (content/docs/automation/flows.mdx on origin/main, one paragraph above the callout I misread):

an object that names a dialect is an expression envelope and must be a valid cel one — a missing, empty or non-string source, or a template / cron dialect, is refused at the variable's path.

⇒ The page already puts a non-string source in the refused bucket, and has for longer than this diff. So it was never a member of the population the later sentence counts — the two shapes that "sit outside what either validator can judge" are the ast-only envelope and the whitespace-only source, both tracked in #15430. ⭐ The count of two is unchanged by this diff, and the exhaustiveness question I raised is moot, exactly as the round said.

My error, stated plainly so it is not repeated: I reasoned from the shared entry (validateExpression) to a page that documents one specific path into it, without checking whether that path has a shape guard in front. It does — AssignmentValueSchema, which the callout names. The crash this PR removes lived on the paths with no shape guard ahead of the shared entry (hooks, sharing rules, action visible/disabled, formula field expressions). ⭐ That is also precisely why formulas.mdx, which documents one of those, is the page that moved, and flows.mdx, which documents a guarded one, is not.

Verdict on the record: formulas.mdx corroborated (its "located, schema-aware" claim was false for that input class before this PR and is true after), flows.mdx untouched, neither falsified, ⛔ no docs edit owed in this PR — rewriting a page this change does not falsify would be the widening E3 forbids.

⭐ Endorsed for the record: the dispatch marked this seat's reading explicitly non-binding and told the round to re-derive rather than adopt it. It did, drove both claims on both sides of the fix with a byte-identical control, and caught the seat. ⛔ That is the protocol working, and a round that had deferred to the PM here would have shipped a wrong verdict.

⛔ Nothing else changes: the PR stays draft and unarmed, Clause-②: yes with needs:contract-review verified on both carriers, awaiting the contract-review tier.

---

Generated by Claude Code

[os-zhuang]

Contract review (clause ②) PASS at head c3fd1902 — director seat, session_01TezFG8ZMrNH6n5VTNpPpdH, 2026-09-05T22:3xZ. Verdict with derived judgments (guard at the shared entry, accept set moved only for the crash class, published surface 39 → 39, caller sweep confirmed), semver and boundary flags: objectstack#15663 comment 5555207174. needs:contract-review removed from this PR and the card in the same stroke (legitimate clear, both citing that comment). Landing per landing-operations.md: CI 31 success / 6 skipped on c3fd1902, governed 0 of 4 ⇒ ready + auto-merge (SQUASH) follow.

Implemented-by: session_01ARYe3yQTQCUFm5qPYNgKaJ
Reviewed-by: session_01TezFG8ZMrNH6n5VTNpPpdH

---

Generated by Claude Code