Repository navigation
Flow node filters silently blank date macros: the template engine consumes {…} before the query engine sees it #3810
Description
Activity
- added a commit that references this issue
on Jul 28, 2026 动手前先实测,发现严重性比我开这个 issue 时描述的高得多 —— 而且主要部分与 filter token 无关。修复在 #3831。
我开 issue 时写错的地方
我写的是"bound 被静默丢弃 → 查询少了一个约束"。实际测下来,插值器把未解析的 token 表达为
undefined,而 filter 里一个值为undefined的键等于该条件不存在。当它是唯一条件时,整个 filter 塌缩成{}:写法 插值后 幸存条件 {record.ownr}字段名拼错{}0 ← 匹配所有行 {someInputVar}该次运行没传{}0 ← 匹配所有行 {record.owner.manager}lookup 穿透{}0 ← 匹配所有行 {current_year_start}filter token{}0 ← 匹配所有行 {status:'open', owner:'{record.ownr}'}{"status":"open"}1 ← 从"我的 open"变成"所有 open" {}交给deleteMany就是整张表。一个字段名拼写错误加一个delete_record节点,就能静默清空整个对象。所以这不是"date macro 用不了"的问题,那只是四个成因之一。前三个成因和 filter token 完全无关,今天就存在,而且 issue 标题完全没覆盖到。
顺带查出的第三件事
resolveFilterTokens()(#3582)只接到了读路径。所以同一个 filter 因动词不同选中不同行集:find解析成usr_1,update/delete把{current_user_id}当字面量。一个 flow 用find_records预览、用update_records执行,两者作用于不同的行。这是我上个 PR 的遗漏,#3106 形状下沉一层。采纳的方案
维护者拍板"三件一起修"+"放行给 filter 方言":
- crud 节点在插值抹掉任何作者写下的条件时拒绝执行(判据是丢失而非为空,所以"删除全部"仍可表达);
interpolateFilter()把 filter 位置的所有权归还给 filter 方言 —— 已知占位符原样透传给引擎,flow 变量保留优先级;- 引擎
update/delete补上解析,且在 by-id 快路径认领标量where.id之前。
方向上值得记一笔:3 是 fail-closed(匹配零行),1、2 修的是 fail-open(匹配全部)。后者才是数据破坏。
Generated by Claude Code
Decision note:
get_recordkeeps the hard failureA question came up after this shipped — is refusing the node too strict for
get_record, given a read is not itself destructive, and a failed node aborts the whole run (the engine has no per-node error branch, only flow-levelerrorHandling.retry)? Recording the answer so it isn't re-litigated from scratch.The guard stays a hard failure on all three filter-guarded nodes,
get_recordincluded.A widened read is not a contained read
get_record's output feeds downstream nodes, so the blast radius is the flow's, not the node's:- With
limit ≤ 1the executor callsfindOne. A filter collapsed to{}returns an arbitrary row, and downstream{rec.id}then drivesupdate_record/delete_recordagainst a record the author never selected. The read is the targeting step for a write. - With
limit > 1it callsfind. Feed that into a notify node and the widened match is an outbound send to everyone.
runAs: 'system'makes the filter the only boundaryFLOW_SCHEDULE_RUNAS_UNSCOPED(#1888 / ADR-0049) deliberately steers scheduled flows towardrunAs: 'system', which bypasses RLS. Under that identity the filter is the only remaining scope. A dropped condition there is a cross-tenant read of the full table — a data-exposure incident, not a papercut. The two guardrails are coupled: relaxing this one quietly widens the other.Downgrading to a warning re-opens the fail-open direction
This issue's own framing: (3) was fail-closed (matched zero rows), (1) and (2) were fail-open (matched all), and the fail-open one is what destroys data. A warning is fail-open by construction — the wrong rows still flow downstream and the diagnostic is only found after the fact, in a run trace someone has to go read. The hard failure surfaces on the first execution, and the fix is a one-token edit.
The error/warning split in this repo already draws the line here
lint-flow-patterns.tsreserves warnings for heuristics that can be wrong — date-equality shapes, phantom aggregation keys. "A condition the author wrote was erased" is not a heuristic; it is a determined bug state. No author writes{x}in a filter meaning "match every row whenxis missing." Deterministically-wrong gets an error, possibly-wrong gets a warning — worth preserving as a rule.It matters most for AI-authored metadata
An AI authoring loop only converges on hard signals.
success: trueplus a warning reads as "this flow works," and the typo is baked into the template. A hard failure naming the template and the three likely causes (field name, unset flow variable, lookup hop needingconfig.expand) is a one-round self-repair. The three reproduced causes here — a typo, an input the run never received, and a lookup hop — are all high-frequency AI error shapes. For an AI author this guard is a training signal, not a restriction.Keeping the four verbs consistent has a practical reason too
Authors routinely tune a filter on
get_recordand paste it intodelete_record. "Warns on read, fails on write" is precisely the verb-dependent inconsistency exit (3) of this issue just removed — worth not reintroducing one layer up.What the "too strict" concern gets right, and where it goes instead
Two follow-ups, neither of which requires relaxing the guard:
- Move detection earlier — fix(lint,cli): a filter reference that cannot resolve fails the build, not the run (#3426, #3810) #3861 raises
validateFlowTemplatePathstoerrorfor filter positions and makesos validategate on it, so the runtime refusal becomes a rarely-hit backstop instead of the first notification. - Give legitimately-optional conditions an explicit syntax — "filter by campaign if one was passed, otherwise return all" is a real need, but the answer is opt-in (an
$optionalmarker, or branching to twoget_recordnodes), with fail-closed staying the default. Filed as Explicit syntax for a legitimately optional filter condition (design only — no implementation until a real request) #3862.
A node-level error branch (#3863) would independently lower the cost of strict guards by letting a flow handle the refusal instead of aborting.
Generated by Claude Code
- With
- added 7 commits that reference this issue
on Jul 28, 2026 - added a commit that references this issue
on Sep 1, 2026 - added a commit that references this issue
on Sep 17, 2026 - added a commit that references this issue
on Sep 29, 2026
Found while implementing #3582 (PR #3809). Out of scope there; filing rather than widening that PR.
What happens
A flow node's
config.filteris run through the flow template interpolator (service-automation/src/builtin/template.ts) before it reaches the data engine. That interpolator owns{…}in this position, and a whole-string token it cannot resolve becomesundefined:interpolateString('{current_year_start}', …)takes the single-token fast path, finds no flow variable of that name, and returnsundefined. The engine then receives{ close_date: { $gte: undefined } }.{30_days_ago}fails the same way by a different route: it does not match the dotted-path grammar, so it falls into the arithmetic branch,_days_agois substituted withnull, andnew Function('return (30null)')throws — alsoundefined.Either way the bound silently disappears. No warning at registration, none at run time.
Why it matters now
Before PR #3809 nothing resolved filter placeholders server-side at all, so this was one instance of a general gap. That PR makes
{current_year_start}/{current_user_id}work everywhere else on the server — the ObjectQL read path and the analytics dataset executor — which leaves flow filters as the one server-side surface where a filter token is still silently dropped. That inconsistency is worse than the uniform gap was: an author who has just learned the tokens work will reasonably try one in a flow.A one-line caveat now sits in
skills/objectstack-query/rules/filters.md, but documentation is a poor substitute for a diagnostic here — the failure mode is exactly the silent-zero class #3574 and #3582 were about.Repro
Expected: rows closing this year. Actual: the bound is dropped, every row matches, no diagnostic.
Options
Two dialects meet in this one config slot, so the decision is which owns it — worth deciding deliberately rather than by accident of evaluation order:
Pass through — the interpolator leaves a whole-string token alone when
isKnownFilterToken()accepts it and no flow variable shadows the name, lettingresolveFilterTokens()handle it downstream. Makes flow filters consistent with every other surface. The cost is one cross-dialect exception, narrowly scoped to filter-bearing config keys.Reject at registration — extend
validateFlowExpressions/validate-flow-template-pathsto flag a filter-token spelling inside a flow filter, telling the author to compute the bound in an earlier node. Keeps the dialects separate; the token stays unsupported, but loudly.Either beats today's silence. (1) is what an author expects; (2) is the cheaper and more conservative change. Flagging the trade-off rather than picking one, since it is a contract decision about who owns
{…}in a flow config.