Skip to content

Commit 2cd6525

Browse files
committed
fix(objectql,skills): hook condition 的 CEL 作用域补上 previous —— 过渡语义可写,与 validation 谓词对齐 (#4784)
声明式 hook 的 `condition` 只绑定一个根 `record`,但两处已发布的 skill 文档 (`objectstack-formula` §5 与它的 `ISCHANGED(x)` → `previous.x != record.x` 迁移条目)一直教作者写 `previous.*`。照着写下去只会静默失效: `No such key: previous` → 被 catch 成 `false` → hook 不触发,只留一条 warn。 declared ≠ delivered。 #4770 之后这条缺口变成了能力缺口:`record` 现在表示记录的**状态**, `record.done == true` 对每一次已完成任务的 update 都为真。「刚刚变成 done」 只能靠比较 `previous` 表达,而 `showcase_audit_task_completion` 的 description 写的正是 "after a task transitions to done"。 - `hook-wrappers.ts`:条件求值绑定 `record` + `previous` 两个根。`previous` 复用 `materializeDeclaredFields`(#4649/#4770 的同一个 helper)对**已声明字段** 做成总全 —— driver 没返回的列读作 `null` 而不是让整条表达式 fault;未声明的 key 仍然不可求值,拼写错误照旧报出来。**拷贝而非原地修改**:`ctx.previous` 是引擎自己的 pre-image,after hook 观察的就是它,物化出的 null 不回灌。 - 取不到 prior 时 `previous` **不绑定**(CEL 里就是一个未声明标识符),与 `validation/rule-validator.ts` 逐字一致:insert 事件没有前态;predicate (`multi: true`) 批量更新一次匹配 N 行、hook 只触发一次,没有单一前置记录可绑。 绑 `{}`/`null` 等于替没人读过的行编造事实。 - **不新增按需取数机制**:`previous` 搭的是 `engine.update` 既有的那一次 prior 取数(注册了 afterUpdate hook 就会取),即喂 `ctx.previous` 和 record-change flow trigger 的同一行。不引用 `previous` 的条件零额外取数, 已用测试钉死。engine.ts 那处 gate 留了注释:今后若收窄它,必须把 hook 条件的 `previous` 需求算进新的判定。 - 文档:`objectstack-automation/SKILL.md` 速查表里的 `ctx.record` 是纯错 (`HookContext` 声明的是 `input` / `result` / `previous` / `session` / `ql`), 改为区分 handler 的 `ctx.*` 与 condition 的 CEL 根;`objectstack-formula` §5 保留 `previous` 示例并补上绑定范围/总全/`!= null` 而非 `has()`/成本说明; `objectstack-data/references/data-hooks.md` 的 condition 一节同步。 - showcase 的 `showcase_audit_task_completion` 改用过渡条件,让它的 description 与实际行为一致。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ
1 parent 84b6e58 commit 2cd6525

8 files changed

Lines changed: 688 additions & 19 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
'@objectstack/objectql': minor
3+
---
4+
5+
**A declarative hook `condition` can now express a TRANSITION: the CEL scope binds `previous` alongside `record` (#4784).**
6+
7+
The condition gate evaluated against a single root — `{ record }`. Both published skill
8+
docs, however, taught the `previous` form: `objectstack-formula` §5 ("Update hook
9+
condition — `previous` vs `record`") gives
10+
`P\`previous.status != 'escalated' && record.status == 'escalated'\``, and its legacy
11+
migration table maps `OLD.x``previous.x` and `ISCHANGED(x)``previous.x != record.x`.
12+
Written into a hook, any of those aborted the expression with `No such key: previous`,
13+
which the gate swallowed into `false` — the hook simply never ran, leaving one WARN line.
14+
Declared ≠ delivered.
15+
16+
It became load-bearing with #4770. `record` now means the record's **state** (stored ⊕
17+
payload), so `record.done == true` is true on *every* update of an already-done row — not
18+
only the one that completed it. `showcase_audit_task_completion`'s own description says
19+
"after a task transitions to done", and there was no way to write that. Now there is:
20+
21+
```ts
22+
condition: P`previous.done != true && record.done == true`
23+
```
24+
25+
`previous` is built exactly as the validation side builds it (#4649), through the shared
26+
`materializeDeclaredFields` helper, so one CEL expression means one thing on both
27+
surfaces:
28+
29+
- **the stored pre-write row**, made **total over the object's DECLARED fields** — a
30+
column the driver never returned reads as `null` instead of aborting the expression;
31+
- **declared fields only**`previous.dnoe` stays unevaluable, so a typo is still
32+
reported rather than quietly answered;
33+
- **copied, never mutated in place.** `ctx.previous` is the engine's own pre-image object,
34+
observed by every after-hook; the materialised `null`s do not leak into it.
35+
36+
**Where `previous` is NOT bound** — verbatim the rule `validation/rule-validator.ts`
37+
already applies, so referencing it there makes the condition unevaluable:
38+
39+
- **insert events** (`beforeInsert` / `afterInsert`) — there is no prior state. Write
40+
insert conditions over `record` alone.
41+
- **predicate (`multi: true`) bulk updates** — one write matches N rows and the hook fires
42+
once, so there is no single prior record. Binding `{}` or `null` would answer
43+
`previous.x == null` with a fabricated fact about rows nobody read.
44+
45+
**Cost: none.** No new demand-driven fetch was introduced. `previous` rides on the prior
46+
row `engine.update` already reads whenever an afterUpdate hook is registered — the same
47+
one that feeds `ctx.previous` and record-change flow triggers. A condition that never
48+
mentions `previous` reads nothing extra, pinned by test.
49+
50+
**What you may see after upgrading:** hooks whose condition referenced `previous` never
51+
fired before and start firing now. That is the declaration finally being honoured — review
52+
any hook carrying a `previous.*` condition before you upgrade.
53+
54+
**Unchanged, deliberately:** a condition that is *still* unevaluable is logged at WARN and
55+
treated as `false`. Whether that should fail loudly instead is tracked separately.

examples/app-showcase/src/data/hooks/index.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,20 @@ export const NormalizeTaskTitleHook = {
3838
description: 'Trims leading/trailing whitespace from the task title before every write.',
3939
};
4040

41-
/** afterUpdate (gated) — log a line whenever a task flips to done. */
41+
/**
42+
* afterUpdate (gated) — log a line on the update that flips a task to done.
43+
*
44+
* The condition compares against `previous` on purpose (#4784). Since #4770
45+
* `record` means the record's STATE, so `record.done == true` alone would audit
46+
* every later edit of an already-done task — while this hook's own description
47+
* says "transitions to done". The transition is the two-root form.
48+
*/
4249
export const AuditTaskCompletionHook = {
4350
name: 'showcase_audit_task_completion',
4451
label: 'Audit Task Completion',
4552
object: 'showcase_task',
4653
events: ['afterUpdate'] as LifecycleEvent[],
47-
condition: "record.done == true",
54+
condition: "previous.done != true && record.done == true",
4855
body: {
4956
language: 'js' as const,
5057
source: "var r = ctx.result || ctx.input || {}; ctx.log.info('task completed: ' + (r.title || r.id || 'unknown'));",

packages/objectql/src/engine.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4495,6 +4495,18 @@ export class ObjectQL implements IObjectQLEngine {
44954495
// record-change flow triggers work: their start-condition gate reads
44964496
// `previous.*` (e.g. `status == "done" && previous.status != "done"`),
44974497
// which silently fails when `previous` is absent.
4498+
//
4499+
// [#4784] It is ALSO what supplies the `previous` binding to a
4500+
// declarative hook `condition` (`hook-wrappers.ts`), which is how a
4501+
// TRANSITION is expressed there: `previous.done != true &&
4502+
// record.done == true`. Note this needs NO second demand-driven
4503+
// fetch — the existing gate already fetches whenever an afterUpdate
4504+
// hook exists, and afterUpdate is the event whose context carries
4505+
// `previous`. Deliberately: adding a "does the condition reference
4506+
// `previous`?" analysis on top would be dead code today. If this
4507+
// gate is ever NARROWED (e.g. scoped per object), hook conditions
4508+
// reading `previous` must be counted into the new demand test —
4509+
// pinned by `hook-condition-previous-scope.test.ts`.
44984510
let priorRecord: Record<string, unknown> | null = null;
44994511
const updateSchema = this._registry.getObject(object);
45004512
const mediaValueShapeStrict = await this.mediaValueShapeStrictFor(updateSchema);

0 commit comments

Comments
 (0)