Skip to content

Commit c0068ed

Browse files
docs(skills, spec): guard the nullable amount in the approval-flow and hook-condition examples (#31322)
<!-- claude-projects-attribution: {"github_login":"huangyiirene"} --> _Requested by **Irene** · [project thread](https://claude.ai/code/project/chan_01TJXtQ1tVD26ecLDZxFS67x?thread=cmsg_01TJXtQ1tVD26ecLDZxFS67x3jwBh95FgzuiJbPAUzCLYc)_ Fixes #31206 Clause-②: no Metadata-protocol: none `dispatch-gates --tier` reads a contract-review model mandate for `skills/**` (clause ①). That mandate is waived by the maintainer's standing order in this project (2026-11-03). This card ran on the project's default model, and the dispatching seat (domain:skills#3) runs the contract review. **Cross-seat note.** `packages/spec/src/data/hook.zod.ts` (the `HookSchema.condition` `@example` and `.describe()` text) and its generated `content/docs/references/data/hook.mdx` row are spec-lane text. They ride along on this skills card under triage 6710970216, so the generated reference and the skill change together. ## 维护者速读 - 改了什么:`objectstack-automation` 技能的审批流示例,把开始节点条件改成 `record.amount != null && record.amount > 100000`;`HookSchema.condition` 的 `@example` 与 `.describe()` 改成 `record.status == 'closed' && record.amount != null && record.amount > 1000`,并用 `gen:docs` 重新生成 `hook.mdx`(只变这一行)。 - 为什么改:发布期空值守卫规则(`packages/lint/src/validate-null-guards.ts`)会拒绝对可空字段做未守卫的大小比较,覆盖 hook `condition` 与 record_change 流程的开始/连线/分支条件;这两处示例照抄即被拒(#31206)。 - 风险与回滚:纯文档示例文字;spec 只改描述文本,不改 schema 形状,不移动任何接受集;技能文件 5727 / 5730 tokens,未动 ratchet 脚本;发版 `@objectstack/skills` 与 `@objectstack/spec` 各 patch;回滚即 revert。 - 先看:`state-machines-and-approvals.md` L90;然后 `hook.zod.ts` L524 与 L526。 - 两分钟验证:`node scripts/check-skills-token-ratchet.mjs`(5727 / 5730);`pnpm --filter @objectstack/spec run check:docs`(hook.mdx 与源一致);对照 `validate-expressions.ts` L2967-L2976 读「开始条件证明非空即在所有连线上被推定」。 - 需要你做的:`check-governed-merges` 判为 Tier H(`skills/**`、`packages/spec/**`),需 `os-zhuang` 或 `hotlong` 批准;请在席位契约复核 ACCEPT 后再批。 ## What changes ### Guarded sites, before and after | Site | Before | After | |:--|:--|:--| | `skills/objectstack-automation/references/state-machines-and-approvals.md` L90, start `config.condition` | ``cel`record.amount > 100000` `` | ``cel`record.amount != null && record.amount > 100000` `` | | same file L128, edge `e1` | ``cel`record.amount > 100000` `` | unchanged: presumed guarded by the start condition | | same file L133, edge `e4` | ``cel`record.amount > 500000` `` | unchanged: presumed guarded by the start condition | | same file L135, edge `e5` | ``cel`record.amount <= 500000` `` | unchanged: presumed guarded; a null `amount` never launches the flow, so it reaches neither branch | | `packages/spec/src/data/hook.zod.ts` L524, `@example` | `"record.status == 'closed' && record.amount > 1000"` | `"record.status == 'closed' && record.amount != null && record.amount > 1000"` | | `packages/spec/src/data/hook.zod.ts` L526, `.describe()` | ``P`record.status == "closed" && record.amount > 1000` `` | ``P`record.status == "closed" && record.amount != null && record.amount > 1000` `` | | `content/docs/references/data/hook.mdx` L39 | generated from L526 | regenerated by `pnpm --filter @objectstack/spec gen:docs` | The guard is the form `skills/objectstack-formula/SKILL.md` teaches (≈L112-L122, `record.x != null && …`), the same form PRs #31127 and #31201 used. The rule is not restated in the automation skill. **Nullability.** The example does not declare `opportunity`. The CRM example's `opportunity.amount` (`examples/app-crm/src/objects/opportunity.object.ts` L48) is `Field.currency({ label: 'Amount', min: 0 })`, with no `required`, no `defaultValue`, so it is nullable and the rule applies. Declaring it `required: true` was not needed. ### Regenerated `hook.mdx` diff (the only change in the generated tree) ```diff -| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL); hook runs only when TRUE. e.g. P`record.status == "closed" && record.amount > 1000` | +| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL); hook runs only when TRUE. e.g. P`record.status == "closed" && record.amount != null && record.amount > 1000` | ``` ## Why The publish-time null-guard rule (`packages/lint/src/validate-null-guards.ts`, wired by `validate-expressions.ts`) refuses an ordering (`< <= > >=`) or arithmetic operator on a declared-nullable field unless an explicit `!= null` / `== null` / `isBlank()` test dominates it in the same boolean branch. It gates the lifecycle hook `condition` and, on a `record_change` flow, the start, edge and `decision` conditions. Both examples compared `record.amount` without a guard, so metadata written from them is refused at `os build` / `os validate` / `os lint` / the runtime publish gate. ## Deviations 1. **Narrowed from four sites to one (seat ruling).** The order listed the start condition and edges `e1`, `e4` and `e5`. On `main`, what the start condition proves non-null is presumed at every edge and `decision` branch, so only the start condition is guarded, spelled with `record.` so both doors accept it: - `packages/lint/src/validate-expressions.ts` L2967-L2976: `provenNonNullKeys(startConditionSource, …)` is passed as `presumed` to every after-start edge and branch check; - `packages/lint/src/validate-null-guards.ts` L270-L273: "The start condition gates every run … what it proves non-null is presumed at every edge and `decision` branch of the flow; an author does not re-guard downstream."; - `skills/objectstack-formula/SKILL.md` L404: "What the start condition proves non-null is presumed at every edge and branch; spell that guard `record.budget != null`". `e5`'s `<=` stays honest: a null `amount` fails the start condition, so the flow never launches and neither `e4` nor `e5` is reached. 2. **No "edges inherit the start guard" clause.** The seat allowed one only if a same-file cut paid for it. The formula skill already teaches it (L404), so none was added and nothing was cut. ## Token readings (`scripts/check-skills-token-ratchet.mjs`, tokens = ceil(bytes / 4)) | File | `main` | This PR | Ceiling | |:--|--:|--:|--:| | `skills/objectstack-automation/references/state-machines-and-approvals.md` | 22883 B = 5721 | 22908 B = 5727 | 5730 | Growth is +25 bytes (`record.amount != null && `), which fits inside the headroom. **No deletions, so no deletion homes.** `scripts/check-skills-token-ratchet.mjs` is not touched (re-measured after merging `origin/main` `f8c3ecc4`: still 5727 / 5730). ## Eval cases touched None. Swept `skills/objectstack-automation/evals/*.json` for `amount`, `100000`, `500000`, `budget` and approval conditions. `flows-triggers-approvals.json` case 3 (`crm_budget_approval`, "when `budget` increases past 100000") asserts only the approval shape (`type: 'approval'`, `approval_revise`, `type: 'back'`, `maxRevisions`, `revise`). No `must_contain` or `expected_output` spells an unguarded comparison, so no case rewards the unguarded form. ## Verification - `packages/spec/src/data/hook.test.ts`: 1 file, **96 / 96 passed**. - `check:skill-examples`, `check:skill-identifier-liveness`, `check:skill-refs`, the token ratchet and `check:docs`: exit 0 (in the gate list below). The approval fence carries no `<!-- os:check -->` marker, so `check:skill-examples` does not type-check it. - `node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD`: exit 0. ### Gates On head `0faa19a1` (merge base `f8c3ecc4`), after a full `pnpm build` (exit 0), under `NODE_OPTIONS=--max-old-space-size=8192`: **all 112 commands from `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` exit 0** (79 pnpm, 33 direct node). The same 112 also all exited 0 on the pre-merge head `247bd53c` (base `85c01247`). The derived list is identical on both heads. Changed lines: 24 (+20 / -4), 4 files. <details><summary>Each command with its exit code (head 0faa19a)</summary> ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-adr-links.mjs :: exit 0 node scripts/check-adr-links.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-adoption.mjs :: exit 0 node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-dev-prereqs.mjs --self-test :: exit 0 node scripts/check-doc-route-spelling.mjs --advisory :: exit 0 node scripts/check-doc-route-spelling.mjs --self-test :: exit 0 node scripts/check-docs-section-name.mjs :: exit 0 node scripts/check-docs-section-name.mjs --self-test :: exit 0 node scripts/check-dts-emitted.mjs --self-test :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-issue-citations.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0 node scripts/check-section-landing-index.mjs :: exit 0 node scripts/check-section-landing-index.mjs --self-test :: exit 0 node scripts/check-skills-token-ratchet.mjs :: exit 0 node scripts/check-skills-token-ratchet.mjs --self-test :: exit 0 node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0 node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0 node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0 node scripts/ci/snapshot-next.mjs --self-test :: exit 0 node scripts/docs-audit/check-affected-docs.mjs :: exit 0 node scripts/docs-audit/check-drift-comment.mjs :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --filter @objectstack/lint run check:doc-formula-expressions :: exit 0 pnpm --filter @objectstack/lint run check:doc-security-posture :: exit 0 pnpm --filter @objectstack/spec run check:api-surface :: exit 0 pnpm --filter @objectstack/spec run check:authorable-surface :: exit 0 pnpm --filter @objectstack/spec run check:browser-reachable-entries :: exit 0 pnpm --filter @objectstack/spec run check:docs :: exit 0 pnpm --filter @objectstack/spec run check:dual-source-exports :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm --filter @objectstack/spec run check:empty-state :: exit 0 pnpm --filter @objectstack/spec run check:entry-nameability :: exit 0 pnpm --filter @objectstack/spec run check:error-code-provenance :: exit 0 pnpm --filter @objectstack/spec run check:export-origins :: exit 0 pnpm --filter @objectstack/spec run check:exported-any :: exit 0 pnpm --filter @objectstack/spec run check:generated :: exit 0 pnpm --filter @objectstack/spec run check:liveness :: exit 0 pnpm --filter @objectstack/spec run check:llms-txt :: exit 0 pnpm --filter @objectstack/spec run check:skill-examples :: exit 0 pnpm --filter @objectstack/spec run check:skill-refs :: exit 0 pnpm --filter @objectstack/spec run check:strictness-ledger :: exit 0 pnpm --filter @objectstack/spec run check:variant-docs :: exit 0 pnpm --filter @objectstack/spec run check:yaml-examples :: exit 0 pnpm check:adr-symbol-anchors :: exit 0 pnpm check:agent-test-spelling :: exit 0 pnpm check:authz-resolver :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:ci-filter-parity :: exit 0 pnpm check:cli-command-ids :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:corpus-claim-drift :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:doc-anchors :: exit 0 pnpm check:doc-authoring :: exit 0 pnpm check:doc-frontmatter :: exit 0 pnpm check:docs-audit-scope :: exit 0 pnpm check:docs-redirects :: exit 0 pnpm check:docs-single-h1 :: exit 0 pnpm check:docs-spec-enumerations :: exit 0 pnpm check:docs-transcript-drift :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:dts-closure :: exit 0 pnpm check:dual-build-cjs-loads :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:issue-citations :: exit 0 pnpm check:keyed-text-bounds :: exit 0 pnpm check:lean-entry-closure :: exit 0 pnpm check:logger-receiver-detach :: exit 0 pnpm check:merge-driver :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:object-def-param-keys :: exit 0 pnpm check:optional-error-sink :: exit 0 pnpm check:org-identifier :: exit 0 pnpm check:page-declaration-shape :: exit 0 pnpm check:platform-object-tenancy-census :: exit 0 pnpm check:pm-governed-merges :: exit 0 pnpm check:pm-prior-rulings :: exit 0 pnpm check:pm-skill-id-lint :: exit 0 pnpm check:published-files :: exit 0 pnpm check:published-readme-links :: exit 0 pnpm check:react-page-adapter-contract :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:registry-log-declared :: exit 0 pnpm check:rest-log-spy-declared :: exit 0 pnpm check:role-word :: exit 0 pnpm check:settings-bind-window :: exit 0 pnpm check:skill-compatibility :: exit 0 pnpm check:skill-frame-sync :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:slot-lookup :: exit 0 pnpm check:sourcemap-no-sources-content :: exit 0 pnpm check:spec-docblock-symbol-anchors :: exit 0 pnpm check:spec-parsed-alias :: exit 0 pnpm check:system-context-census :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:vendor-version-stamps :: exit 0 pnpm check:vouch-for-reference-callers :: exit 0 pnpm check:watch-hint-literal :: exit 0 ``` </details> The NOT MEASURED families `dispatch-gates` names (CI-only steps, type-check lanes, the dogfood and build jobs) run in CI. ## Other sites found (not changed here, for the seat to file) Grep over `content/docs/**` and `skills/**` (release notes and CHANGELOGs excluded) for `record.amount` under an ordering or arithmetic operator: | Site | Surface | Gated? | |:--|:--|:--| | `content/docs/getting-started/common-patterns.mdx` L371-L372, edges `e2`/`e3` (`record.amount < 100` / `>= 100`) | `record_change` flow on `expense_report`. `amount` is declared `{ type: 'currency' }` with no required/default (L312), and the start condition does not guard it | **Yes, refused as written** | | `content/docs/automation/approvals.mdx` L145, start `condition` (`record.amount >= 50000 && …`) | `record_change` flow start condition on `opportunity` | **Yes**, if `amount` is nullable (the snippet doesn't declare the object; the CRM example's is nullable) | | `content/docs/automation/hook-bodies.mdx` L74 (`condition: 'record.status == "open" && record.amount > 1000'`) | hook `condition` (prose example) | **Yes**, if `amount` is nullable (object not declared) | | `content/docs/data-modeling/formulas.mdx` L119, L356 (``requiredWhen: P`record.amount > 10000` ``) | field `requiredWhen` | **Yes** (covered surface; fail-open, so it would silently enforce nothing), if `amount` is nullable | | `content/docs/data-modeling/formulas.mdx` L394 (``condition: P`record.amount <= 0` ``) | validation rule | **Yes**, if `amount` is nullable (object not declared) | | `content/docs/data-modeling/validation.mdx` L51 (`record.amount <= 0`) | validation rule | No: `amount` is `required: true` in that example (L31) | | `content/docs/data-modeling/validation.mdx` L329 (`record.amount <= 0`) | validation rule (fragment, no object) | Yes, if `amount` is nullable | | `content/docs/automation/flows.mdx` L2473 (table cell, `record.amount > 500` for a start-node `condition`) | flow start condition (syntax table) | Yes on a `record_change` flow, if nullable | | `content/docs/references/data/validation.mdx` L170/L291/L505/L571 (``P`record.amount < 0` ``) | validation `condition`, generated from `packages/spec/src/data/validation.zod.ts` L180 | Yes, if nullable (spec lane, generated) | | `content/docs/permissions/permissions-matrix.mdx` L179, L199 | sharing-rule `condition` | No: compiled to SQL, excluded by the rule's ledger | | `content/docs/data-modeling/formulas.mdx` L491-L493, L577; `content/docs/automation/flows.mdx` L543; `skills/objectstack-data/rules/field-types.md` L203; `skills/objectstack-formula/SKILL.md` L57; ``F`record.amount * 0.1` `` in generated `field.mdx` / `object.mdx` / `migration.mdx` | formula `expression` / node value envelope | No: formula expressions are excluded by product judgement; node values are not judged | | `skills/objectstack-automation/references/state-machines-and-approvals.md` L128, L133, L135 | flow edges | No longer refused: presumed by the guarded start condition (this PR) | Also noted: `ui/packages/app-shell/src/views/metadata-admin/SchemaForm.erasedArmMark-9830.test.tsx` L110 holds a hand-copied fixture of the served `hook.condition` description, including the old example text. The test reads its own constant, not the spec, so it still passes. The fixture will lag the served text once this lands, and it sits outside this card's surface. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic (description text only). - [x] 2. No names added. - [x] 3. No formatter ran. - [x] 4. One issue, one branch, one pull request (`origin/main` merged in, no rebase). - [x] 5. Changeset `.changeset/31206-approval-and-hook-amount-null-guards.md`: `'@objectstack/skills': patch` + `'@objectstack/spec': patch`, `Clause-②: no`, `Metadata-protocol: none`. - [x] 6. `packages/spec` generated artifacts regenerated and committed (`hook.mdx`). - [x] 7. `content/docs/references/data/hook.mdx` regenerated by the generator, not hand-edited. - [x] 8. English body (the 维护者速读 block is the lane's Chinese summary). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J8VVAkApzF5rsWNU6MwSN3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7189fb2 commit c0068ed

4 files changed

Lines changed: 20 additions & 4 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/skills': patch
3+
'@objectstack/spec': patch
4+
---
5+
6+
docs(skills, spec): the approval-flow example and the `HookSchema.condition` example guard their nullable `amount` comparison with `record.amount != null`
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
The publish-time null-guard rule refuses an ordering comparison on a declared-nullable field in a hook `condition` and in a record-triggered flow's start, edge and `decision` conditions unless an explicit `!= null` test dominates it. Two published examples compared `record.amount` without one:
12+
13+
- `skills/objectstack-automation/references/state-machines-and-approvals.md`: the approval flow's start condition now reads `record.amount != null && record.amount > 100000`. What the start condition proves non-null is presumed at every edge and `decision` branch, so the `e1`, `e4` and `e5` edge conditions pass as written, and a null `amount` never launches the flow.
14+
- `packages/spec/src/data/hook.zod.ts`: the `condition` `@example` and `.describe()` text now read `record.status == 'closed' && record.amount != null && record.amount > 1000`, and the generated `content/docs/references/data/hook.mdx` row follows.
15+
16+
The spec change is description text only: no schema shape changes, and no accept set moves.

‎content/docs/references/data/hook.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const result = HookSchema.parse(data);
3636
| **body** | `{ language: 'expression'; source: string } \| { language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Hook body — expression (L1) or sandboxed JS (L2) |
3737
| **priority** | `number` | optional (default: `100`) | Execution priority |
3838
| **async** | `boolean` | optional (default: `false`) | Run specifically as fire-and-forget |
39-
| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL); hook runs only when TRUE. e.g. P`record.status == "closed" && record.amount > 1000` |
39+
| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source: string; ast?: any; meta?: object }` | optional | Predicate (CEL); hook runs only when TRUE. e.g. P`record.status == "closed" && record.amount != null && record.amount > 1000` |
4040
| **description** | `string` | optional | Human-readable description of what this hook does |
4141
| **retryPolicy** | `{ maxRetries?: number; backoffMs?: number }` | optional | Retry policy for failed hook executions |
4242
| **timeoutMs** | `number` | optional | Maximum execution time in milliseconds before the hook is aborted |

‎packages/spec/src/data/hook.zod.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,9 @@ export const HookSchema = lazySchema(() => strictObject(
521521
* If provided and evaluates to FALSE, the hook is skipped entirely.
522522
* Useful for filtering by record data without writing handler code.
523523
*
524-
* @example "record.status == 'closed' && record.amount > 1000"
524+
* @example "record.status == 'closed' && record.amount != null && record.amount > 1000"
525525
*/
526-
condition: EvaluatedExpressionInputSchema.optional().describe('Predicate (CEL); hook runs only when TRUE. e.g. P`record.status == "closed" && record.amount > 1000`'),
526+
condition: EvaluatedExpressionInputSchema.optional().describe('Predicate (CEL); hook runs only when TRUE. e.g. P`record.status == "closed" && record.amount != null && record.amount > 1000`'),
527527

528528
/**
529529
* Human-readable description

‎skills/objectstack-automation/references/state-machines-and-approvals.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ is one diagram a reviewer (or AI) can read end-to-end.
8787
config: {
8888
objectName: 'opportunity',
8989
triggerType: 'record-after-update',
90-
condition: cel`record.amount > 100000`,
90+
condition: cel`record.amount != null && record.amount > 100000`,
9191
},
9292
},
9393
{

0 commit comments

Comments
 (0)