Skip to content

Commit 18ed2db

Browse files
docs(spec): the FlowSchema example guards its nullable amount comparisons (#31416)
<!-- claude-projects-attribution: {"github_login":"marchtian"} --> _Requested by **田騏** · [project thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiAZaP1vfuzw3guARwSyQaKN)_ Fixes #31367 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么:** `FlowSchema` 文档注释里的审批流示例:开始节点绑定到 `order`(记录更新后触发),自动审批边改为 `record.amount != null && record.amount < 500`,送审边改为默认边(`isDefault: true`)。只改注释文字,外加一个 patch changeset。 - **为什么改:** 原示例对可空的 `amount` 直接做 `<` / `>=` 比较;作者照抄并绑定到金额可选的对象时,发布期空值保护规则会拒收两条边,运行时金额为空会让流程失败。现在金额为空的订单走人工审核,而不是失败或被自动通过。 - **风险与回滚:** 无 schema 形状变化、无接受集变化、无参考页再生成(该示例不进入任何生成产物);回滚即 revert。 - **先看:** `packages/spec/src/automation/flow.zod.ts` 的 `@example Simple Approval Logic`(约 L1048–1078);`.changeset/31367-flow-example-amount-null-guard.md`。 - **两分钟验证:** 打开上述示例,确认 e2 带 `!= null` 保护、e3 为 `isDefault: true`、开始节点有 `config: { objectName: "order", triggerType: "record-after-update" }`。 - **需要你做的:** 无,席位落地。 ## What changes The `FlowSchema` TSDoc `@example` in `packages/spec/src/automation/flow.zod.ts`: - The start node now carries `config: { objectName: "order", triggerType: "record-after-update" }`. Before, it named no object, so the example was never record-triggered (`resolveFlowTriggerKind` reads `start.config.triggerType`) and the null-guard rule never judged it. - Edge `e2` (auto-approve): `record.amount < 500` becomes `record.amount != null && record.amount < 500`, the `!= null &&` idiom from the formula skill and the precedent PR #31358. - Edge `e3` (review): `record.amount >= 500` becomes `isDefault: true` with no condition. It takes every order `e2` does not, including one with no amount. - Three comment lines: on a record-triggered flow an optional field reads `null` when unset, `<` on null aborts the condition, test `!= null` first (`has()` does not); and why `e3` is the default edge. Plus a `'@objectstack/spec': patch` changeset. ### The choice the card asked for: guard the condition, not `required: true` The `FlowSchema` example defines no object, so it has nowhere to declare `required: true`. #31358 chose `required` only where the object was defined in the same snippet (`common-patterns.mdx`). An author copying a flow example binds it to an object they already have, and on that object `amount` is usually optional. So the example teaches the guard they need to write. For the review edge, the schema's own `isDefault` docblock (a few lines above the example) says the "otherwise" branch should be a default edge, not a hand-written negation of every sibling condition. The first draft used `record.amount == null || record.amount >= 500`. That also passes the rule, but the review asked for the default edge, and this version uses it. Routing with the decision node's out-edges (exclusive, first condition that holds, else the default): amount `499` goes to `auto_approve`, while `500` and `null` go to `submit_for_approval`. The routing is exhaustive and exclusive. ## Tests Proof against the real rule: a scratch vitest file in `packages/lint/src` (not committed). It evaluated the `@example` object literal **verbatim** from the file (from `origin/main` for "before", from the branch for "after") and ran `validateStackExpressions` (the gate `os validate`, `os build` and the publish gate run) with `order.amount` declared `{ type: 'currency' }` (nullable): <details><summary>Readings</summary> ```text BEFORE verbatim (start unbound) null-guard findings 0 (the premise: it passes only because the start names no object) BEFORE bound to order(amount optional) null-guard findings 2 - error flow 'approve_order_flow' · edge 'e2' (check_amount→auto_approve) condition :: … applies `<` to `record.amount`, which 'order' declares as nullable … - error flow 'approve_order_flow' · edge 'e3' (check_amount→submit_for_approval) condition :: … applies `>=` to `record.amount`, which 'order' declares as nullable … AFTER verbatim null-guard findings 0 AFTER verbatim ALL expression issues 0 AFTER FlowSchema.safeParse success true ``` </details> I added no permanent test. The guard semantics (`!= null &&`, `== null ||`, `has()` is not a guard) and the edge wiring are already pinned in `packages/lint/src/validate-null-guards.test.ts` and `validate-expressions.flow-null-guards.test.ts`, and this diff changes only doc-comment text. ## Acceptance notes - `gen:schema` + `gen:docs` produce no diff: `build-docs.ts` renders `.describe()` text, not TSDoc `@example`, and no file under `content/docs/references/**` or `packages/spec/json-schema` embeds this example. - Out of scope, the same unguarded idiom elsewhere (no card filed; listed for the seat): `skills/objectstack-automation/references/state-machines-and-approvals.md` (`record.amount > 500000` / `<= 500000`), and `packages/services/service-automation/README.md` (`record.amount > 500`). ## Verification Local gate union on the final head `f6f11f22` (merge of `origin/main` into 8c1cc37), derived by `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: - 79 families derived, 79 run, all `exit 0`. `dispatch-gates --ran`: "79 derived famil(ies) accounted for, 79 run, 0 NOT-MEASURED (a DERIVED zero)". - Prerequisites cleared before the run: a `./packages/**` turbo build (81 tasks, for `check:dual-build-cjs-loads`, `check:lean-entry-closure` and lint's `check:doc-formula-expressions`), plus the fetched positive-control commit for `check-plugin-teardown-shape --self-test`. - After the merge: `pnpm install --frozen-lockfile`, spec `gen:schema` + `gen:docs` (no diff), `check:generated`. Its first run hit a heap-limited `tsc` in `check:test-typecheck` ("failed without diagnostics"). Rerun alone with `NODE_OPTIONS=--max-old-space-size=6144`: `exit 0`, "52 file(s) / 240 error(s) / 131 pinned signature(s) held". All 14 artifacts are current, and the tree is clean. - `eslint --no-inline-config packages/spec/src/automation/flow.zod.ts`: `exit 0`. `node scripts/check-nul-bytes.mjs`: `exit 0`. - The proof (Tests above) was re-run on `f6f11f22` with the same readings. - CI owns the repo-wide `pnpm lint`, `test:repo` and the full check farm. Fable review of the spec diff (the maintainer's standing rule for spec changes): round 1 **PASS with nits**, with two should-fixes: use `isDefault` instead of the hand-written negation, and bind the start node / qualify the comment. Both are fixed in 8c1cc37. The incremental re-review of the delta: **PASS**. The verdict comment is posted on this PR. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed (none changed). - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the repo's own Chinese reader section). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_017ihL2ejruuhep9NDrZsCT7 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ae816c4 commit 18ed2db

2 files changed

Lines changed: 23 additions & 3 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
docs(spec): the `FlowSchema` example guards its nullable `amount` comparisons
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The `FlowSchema` doc-comment example branched on `record.amount < 500` and `record.amount >= 500` with no null test. Bound to an object whose `amount` is optional, as an author copying it would bind it, the publish-time null-guard rule refuses both edges: on a record-triggered flow a null `amount` aborts the condition and the run fails. The example now binds its start node to `order` (`record-after-update`), so the rule judges it as written; the auto-approve edge reads `record.amount != null && record.amount < 500`, and the review edge is the default edge (`isDefault: true`), so every other order, one with no amount included, goes to review.
11+
12+
The spec change is doc-comment text only: no schema shape changes, and no accept set moves.

‎packages/spec/src/automation/flow.zod.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,11 @@ export const FlowEdgeSchema = lazySchema(() => strictObject(
10531053
* type: "record_change",
10541054
* status: "active",
10551055
* nodes: [
1056-
* { id: "start", type: "start", label: "Start", position: {x: 0, y: 0} },
1056+
* { id: "start", type: "start", label: "Start", position: {x: 0, y: 0},
1057+
* // Launch only on the transition into "submitted": without a start condition the
1058+
* // auto-approve write is itself an update and would launch the flow again.
1059+
* config: { objectName: "order", triggerType: "record-after-update",
1060+
* condition: "record.status == 'submitted' && previous.status != 'submitted'" } },
10571061
* { id: "check_amount", type: "decision", label: "Check Amount", position: {x: 0, y: 100} },
10581062
* { id: "auto_approve", type: "update_record", label: "Auto Approve", position: {x: -100, y: 200},
10591063
* config: { objectName: "order", filter: { id: { dialect: "cel", source: "record.id" } }, fields: { status: "approved" } } },
@@ -1068,8 +1072,12 @@ export const FlowEdgeSchema = lazySchema(() => strictObject(
10681072
* // Conditions are bare CEL (ADR-0032). Reference fields directly —
10691073
* // `record.amount`, `previous.status`, `<var>.field` — and DO NOT wrap them
10701074
* // in `{…}` template braces: `{amount}` parses as a CEL map literal and fails.
1071-
* { id: "e2", source: "check_amount", target: "auto_approve", condition: "record.amount < 500" },
1072-
* { id: "e3", source: "check_amount", target: "submit_for_approval", condition: "record.amount >= 500" }
1075+
* // On a record-triggered flow an optional field reads `null` when unset, and
1076+
* // `<` on null aborts the condition: test `!= null` first — `has()` does not.
1077+
* // e3 is the default edge: it takes every order e2 does not, so an order
1078+
* // with no amount goes to review instead of being auto-approved.
1079+
* { id: "e2", source: "check_amount", target: "auto_approve", condition: "record.amount != null && record.amount < 500" },
1080+
* { id: "e3", source: "check_amount", target: "submit_for_approval", isDefault: true }
10731081
* ]
10741082
* }
10751083
*/

0 commit comments

Comments
 (0)