Skip to content

fix(flows): make record-change conditions total so three automations stop silently not running - #644

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-633-flow-condition-totality
Aug 2, 2026
Merged

fix(flows): make record-change conditions total so three automations stop silently not running#644
os-zhuang merged 1 commit into
mainfrom
claude/issue-633-flow-condition-totality

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #633

结论先说:flow 这条路径确实暴露,而且已经在坏了

本 issue 要求把三件事分别实测,不许从 validation 面或 sharing 面继承结论。三件都量了,用的是真 ObjectQL + InMemoryDriver + 真 RecordChangeTrigger + 真 AutomationEngine

1. 记录形状是什么? 既不是重新读取,也不是 {...previous, ...data}RecordChangeTriggerrecord 拼成 { ...input.data, ...ctx.result } —— 变更载荷之上覆盖驱动的写后行。只有当对象声明了 formula 字段时才会把一次重读 merge 到下面(只能补 key,不能填值)。previousctx.previous,即驱动的前一行,同样稀疏,并且在 insert 和批量 updateMany 时为 null。所以在只存"写过的列"的驱动上(driver-memory / driver-mongodb),条件看到的记录是真的缺 key

2. 解释执行还是编译成过滤器? 每次运行都解释执行。AutomationEngine.evaluateCondition 直接把源码交给 ExpressionEngine 当严格 CEL 求值。关键的反证compileCelToFilter 在安装好的平台里只有三个调用方 —— @objectstack/formula(定义它的)、plugin-sharingplugin-securityservice-automationtrigger-record-change 都不碰它。所以 #637 关于 sharing 的结论到 sharing 边界为止,不能外推到这里。

3. abort 之后会怎样? evaluateCondition 抛异常 → AutomationEngine.execute catch 住,把这次运行记成 status: 'failed',返回 { success: false, error } → automation 服务打一条 ERROR:Trigger-fired run of flow '<name>' failed: …触发写入本身照样成功。 所以它不是 validation 那种静默跳过,它是响的 —— 但依然是"声明了 ≠ 生效了":自动化没跑,界面上没有任何提示。

实测到的三个真缺陷(端到端复现,不是推演)

flow 报错 触发场景
case_escalation_on_create No such key: escalated_date 一开就是 critical 的 case 存下来根本没有 escalated_date 这一列 —— 而按它自己文件里的注释,电话报进来的 P1 正是常见路径
contact_welcome No such key: owner owner 的默认值是 os.user.id,在不带用户的写入(种子数据、集成、任何 system context)下求值失败,ObjectQL 打 Failed to evaluate default expression 然后整列不存
lead_assignmente2 No such key: rating rating 既非 required 也无默认值,没评级的 lead 拿不到 SLA 也拿不到提醒

为什么一直没人发现:CEL 的 && 会吸收错误

实测:error && falsefalseerror && true 才 abort。所以未加守卫的条件对每一条本来就该跳过的记录都答得好好的,只在它本该处理的那些记录上炸 —— 这正是最难被发现的形态。

改了什么

每个 record.x / previous.x 读取都加 has(...);所有序关系比较额外加 != null(显式 null 能通过 has(),然后在比较处报 no such overload: dyn<null> > int)。

改写是保守的:在 absent / null / 各种取值的完整笛卡尔积上逐一比对,凡是原表达式给得出答案的形状,新表达式给出同样的答案;原表达式 abort 的形状,新表达式全部给出布尔值。

两处需要判断,已在文件里写明理由:

  • lead_assignment 的两条边必须构成划分。 两边都写 has(...) && 会把一个响亮的 abort 换成静默的空转(没评级的 lead → 没 SLA、没提醒、没报错),正是本仓库反复删规则要消灭的形态。所以 hot 分支要求 rating 可读,standard 分支吸收其余一切 —— 没评级的 lead 本来就不是 hot lead。
  • opportunity_won_alertprevious.stage 走 fail-closedhas(previous.stage) &&,与同一条件里 record.stage 的方向相反,是刻意的)。这一项存在的唯一目的是压制重复触发,而它防的那个事故(赢单后每次编辑都给管理层重发一次祝贺)比漏发一次更糟。引擎看不到前一个 stage 时就看不到"跃迁",那就不该声称有跃迁。

src/sharing/ 一行没碰

按 PM 要求,也按 #637 的实测:sharing 条件是被编译成下推过滤器的,函数调用整类都翻译不了,加 has() 会让 9 条规则全部变成不可翻译并被静默跳过。test/sharing-seeding.test.ts 里的 sharing conditions cannot use has() 就是钉这个的。本 PR 反过来在新测试里钉了 has() evaluates in the flow engine, unlike on the sharing surface —— 两条断言互为对照,谁也别再把结论搬到对面去。

测试

新增 test/flow-condition-totality.test.ts,三层设防:

  1. 结构扫描 —— 每个读取都有 has();每个序比较都有 null 比较(两种极性都接受,因为划分场景需要互补形式)。
  2. 实测扫描 —— 把每条条件喂给真的 AutomationEngine.evaluateCondition,跑"全空记录 / previous 为 null / 全 null / 每次只缺一个 key"四组形状。缺 key 那组的填充值取自条件自身的字面量(并对数值字面量取 n-1 / n / n+1),这样每个探针都是类型正确的 —— 随便填个字符串进 currency 字段测的是类型不匹配,不是全域性;同时这也能穿透上面说的 && 错误吸收。
  3. 端到端 —— 起真 ObjectQL + InMemoryDriver + 真 record-change trigger,用普通写入复现上述三个缺陷,并断言"存下来的行确实缺这一列"这个前提(哪天平台把这个驱动改成列完整,这条断言会先红,提醒后面的人上面的测试已经不证明任何事了)。

把守卫 stash 掉重跑,这个文件红 30+ 条;带守卫跑 56 条全绿。

顺带修了 test/docs-drift.test.ts:manager 审批阈值的抽取正则原本靠"值旁边那个 && (record.approval_status 子句"定位,守卫插进两者之间就失配了。现在改成按 record. 作用域定位(大小写敏感,所以不会误匹配 director 档的 oppRecord.amount)—— drift 检测该锚在值自己的作用域上,而不是碰巧挨着它的东西上。

验证输出

pnpm validate   ✓ Validation passed (806ms)      ← has() 在 flow 条件里平台校验放行
pnpm typecheck  ✓ (no output)
pnpm lint       1 warning(s), 13 suggestion(s)   ← 均为既有项,与本次改动无关
pnpm hygiene    ✓ source hygiene clean
pnpm build      ✓ Build complete (844ms)
pnpm test       Test Files  42 passed (42)
                Tests  996 passed | 1 skipped (997)

顺手记录、未在本 PR 处理

🤖 Generated with Claude Code

https://claude.ai/code/session_019SS7C5SXpniKeCApxgARyf


Generated by Claude Code

…stop silently not running (#633)

Strict CEL aborts a whole condition on a key that is not PRESENT, not merely
one that is null. The record-change trigger builds the flow's `record` as
`{...input.data, ...driverPostWriteRow}` and `previous` from the driver's prior
row, so on driver-memory / driver-mongodb — which store only the columns a row
was written with — an unwritten field arrives genuinely missing.

Measured end-to-end (real ObjectQL + InMemoryDriver + RecordChangeTrigger),
three conditions aborted on ordinary writes:

  case_escalation_on_create  No such key: escalated_date  (case born critical)
  contact_welcome            No such key: owner           (system/seed write)
  lead_assignment edge e2    No such key: rating          (unrated lead)

The abort makes AutomationEngine.execute record the run as failed and log at
ERROR; the triggering write still succeeds, so the automation simply does not
happen. It stayed hidden because CEL's `&&` absorbs an error beside a false
operand: the conditions answered correctly for every record they were meant to
skip and aborted only on the ones they were meant to act on.

Every `record.x` / `previous.x` read now carries `has(...)`, and every ordering
comparison also carries `!= null` (an explicit null passes has() and then
aborts with `no such overload: dyn<null> > int`). The rewrites are conservative:
across the full cross-product of absent/null/valued shapes they return the same
answer as the originals wherever the originals returned one.

Two judgement calls are documented in-file. lead_assignment's two edges must
PARTITION, so the standard branch absorbs an unreadable rating — guarding both
with `has(...) &&` would have traded a loud abort for a silent no-op.
opportunity_won_alert guards `previous.stage` fail-closed, because that term
exists only to suppress a repeat blast to management.

src/sharing/ is deliberately untouched: sharing conditions are COMPILED to
pushdown filters by compileCelToFilter, which rejects the function-call class,
so a has() guard there makes the rule untranslatable and plugin-sharing stops
seeding it (#621 / #637). Verified the flow path never reaches that compiler —
its only consumers are @objectstack/formula, plugin-sharing and plugin-security.

Adds test/flow-condition-totality.test.ts: a structural sweep, a measured sweep
through the real AutomationEngine.evaluateCondition, and end-to-end regressions
for all three defects. It also pins that has() IS valid here, as the explicit
counterpart to sharing-seeding.test.ts's `sharing conditions cannot use has()`.

docs-drift.test.ts's manager-threshold pattern keyed on the clause next to the
value, which the guards displaced; it now keys on the `record.` scope that
distinguishes it from the director tier's `oppRecord.amount`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SS7C5SXpniKeCApxgARyf
@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Aug 2, 2026 4:15pm

Request Review

@github-actions github-actions Bot added ci/cd CI plumbing and the verification pipeline backend Server-side behaviour — hooks, flows, actions labels Aug 2, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 2, 2026 16:16
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 2, 2026
Merged via the queue into main with commit 3dcc301 Aug 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Server-side behaviour — hooks, flows, actions ci/cd CI plumbing and the verification pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flow start conditions carry no has() guards — measure whether the abort-and-skip class of #630 reaches them

2 participants