fix(lint): flow 规则族下钻 loop body 及所有嵌套区域 (#5383) - #5635
Merged
Conversation
…ow rule family (#5383) The flow anti-pattern rules read a flow's `nodes` / `edges` flat off the top level, so every rule in the family was blind to anything authored inside an ADR-0031 container — a `loop` body, a `parallel` branch, a `try_catch` try/catch. Loop bodies are where a lot of real branching lives, so this was a large share of authorable flow metadata that no flow rule inspected. Measured in a real app: 8 `decision` nodes carried the inert singular `config.condition` that `flow-inert-node-condition` exists to catch, all 8 inside a `loop` body, and none were reported. The identical key on a top-level decision fired immediately — same key, same node type, only the nesting depth differed. `lintFlowPatterns` now iterates `collectFlowGraphs` — the same traversal the engine's registration pass uses, and the one `validate-expressions.ts` already uses on the author side — and prefixes each finding's `where` with the region scope, so a message still points at exactly one node. Findings on a flow's own graph are unchanged byte for byte, since the top-level graph's scope is empty. Two properties of the walk are load-bearing rather than incidental: - Nodes and edges stay PAIRED per region. The branch-routing rules reason about a node together with its out-edges, and a region is a self-contained sub-graph. Flattening into one node bag plus one edge bag would break them both ways: a nested decision's out-edges would be absent from the top-level list so it would read as having none and be skipped, while two nodes in different regions sharing an id (ids are unique per graph, not per flow) would have their out-edges merged into one phantom fan-out. A regression test pins the second case. - A container's config is read region-STRIPPED for the recursive scans. `collectTemplateStrings` walks config to its string leaves and a container's config physically contains its descendants', so a nested double-brace hit was already visible before this change — but attributed to the enclosing `loop`, the same failure mode #4380 fixed for `validate-flow-template-paths`. Descending without stripping would have made it a double report. It now names the node carrying the string, still exactly once. `stripRegions` is exported from `flow-walk.ts` rather than copied, so there is one definition of that view. `flow-runas-unscoped` deliberately keeps its top-level-only data-node search: widening a build-gating rule is its own change with its own blast radius, filed as #5633. Verified the repo's own example apps (`app-showcase` / `app-crm` / `app-todo`, 34 flows) report zero findings before and after, and that the descent does reach their real loop bodies — an inert condition injected into showcase's `loop_tasks` body is caught and scoped to it.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
August 5, 2026 21:08
os-zhuang
enabled auto-merge
August 5, 2026 21:08
This was referenced Aug 5, 2026
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Aug 6, 2026
…bjectstack-ai#5482) (objectstack-ai#5663) * feat(lint): warn on a `multi: true` delete/update bounded by nothing (objectstack-ai#5482) `config: { objectName: 'lead', multi: true }` with no `filter` is a WHOLE-OBJECT write: the executor forwards `where: {}` plus the bulk intent, the engine classifies it as a legal `multi` call, and it lands on `deleteMany`/`updateMany` with no predicate. Reachable only since objectstack-ai#5393 gave these nodes a bulk declaration, and silent ever since — the author's only feedback was the step's `acted` count, after the rows were gone. `flow-multi-write-unfiltered` says it at authoring time. A warning, not a gate: the engine's dispatch case-set lists "bulk intent, no predicate" as a valid call, so an explicit purge is an intent the platform grants — which is also why this is not a spec refine. Not a second copy of the objectstack-ai#3810 run-time guard: that one refuses a node when a condition the author WROTE interpolated to nothing, and is deliberately keyed on "a written condition is gone" rather than "the filter is empty". The two judge different facts, and the diagnostic names the other one so they are not mistaken for one check. Reported at every nesting depth (objectstack-ai#5383/objectstack-ai#5635), because a loop-body sweep is the standard janitor shape. Empty combinator arrays are deliberately out of range: objectstack-ai#5322/objectstack-ai#5134 already ruled their identities, and deciding them here would be a fourth hand-written copy of a producer-side reduction. * refactor(lint): cite the right dispatch authority per write node (objectstack-ai#5482) `update_record` has no extracted dispatch module — only delete's is case-set-pinned (`engine-delete-dispatch.ts`). The message used delete's wording for both, which credited update with a case-set that does not exist. Each node type now carries its own authority: delete cites the case-set, update cites the `options.multi` branch (whose refusal fires only when the declaration is absent), and both tests assert their own. --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5383
问题
packages/lint的 flow 反模式规则族从 flow 顶层平铺读nodes/edges,从不下钻 ADR-0031 容器(loop的config.body、parallel的branches[]、try_catch的try/catch)。整族因此对嵌套节点全盲。而"扫一批记录、循环、逐条判断"正是定时 flow 的标准形状——per-item 的 gate 几乎总在 loop 里。issue 的实测:某真实 app 里 8 个
decision带着flow-inert-node-condition专门要抓的 inert 单数config.condition,8 个全在 loop body 内,pnpm lint一个都没报;同样的 key 放到顶层 decision 上立刻报。同 key、同节点类型,唯一区别是嵌套深度。盲区也解释了它自己为何能存活这么久:门禁在看得见的地方确实工作,所以顶层的副本被陆续清掉了,而嵌套的那些"看起来像是被批准过的"。
改法
lintFlowPatterns改为迭代collectFlowGraphs——引擎注册阶段用的同一个遍历(validateNodeConfigKeys/validateFlowExpressions),也是validate-expressions.ts在作者侧已经在用的那一个。没有新建第四份区域遍历:region-slots.ts已经把"区域槽位在哪"收敛成一张表,图级遍历(nodes + edges 成对)按那张表的分工归 spec。每条 finding 的
where前缀加区域 scope,消息仍然唯一定位到一个节点:顶层图的
scope是空串,所以顶层 finding 的文案逐字节不变。覆盖到的规则:
flow-inert-node-condition、flow-decision-unconditional-branch、flow-branch-label-unmatched、flow-default-edge-with-condition、flow-multiple-default-edges、flow-double-brace-interpolation、flow-bare-dollar-reference、flow-date-equality-filter、flow-phantom-aggregation、flow-error-label-not-fault,以及scanApprovalReviseLoops/scanErrorLabelledEdges/scanBranchRouting三个 scan 函数。其中flow-default-edge-with-condition是severity: 'error'——这条修的正是 issue 点出的严重性不对称:一条会挡构建的规则此前看不见下一层写的矛盾。两处是承重的,不是顺手
1. 每个区域的 nodes 与 edges 必须成对。 branch-routing 族要把节点和它的出边一起判断,而区域是自洽子图。拍平成一个 nodes 袋 + 一个 edges 袋会两个方向都坏掉:
outs.length === 0直接跳过——静默漏判;第二种情况有专门的回归测试钉住(两个 parallel branch 各有一个同名
gate各带一条isDefault出边,拍平实现会误报flow-multiple-default-edges,成对实现保持安静)。2. 容器自己的 config 在递归扫描时要剥掉区域槽位。
collectTemplateStrings递归到字符串叶子,而容器的 config 物理上包含所有后代的 config。这里有个反直觉的事实:改动前 loop body 里的{{ }}其实已经被看见了,只是被归属到外层loop节点上——正是 #4380 给validate-flow-template-paths修过的同一种失败模式(看得见,但判在了错误的节点上)。所以如果只下钻而不剥离,就会变成报两次。剥离后 finding 落到真正携带该字符串的节点上,且仍然只报一次。stripRegions从flow-walk.ts导出复用,没有复制第二份——"config 去掉区域"这个视图在包内只有一个定义。边界
@objectstack/service-automation:引擎侧本来就是对的,本 PR 只让 linter 追上它。flow-runas-unscoped故意保留只搜顶层节点的行为:它也有同族盲区(实测顶层报 1 条 error、挪进 loop body 报 0 条),但它是会挡构建的 gating 规则,放宽它的爆炸半径需要单独决策,已另开flow-runas-unscoped(severity error) still searches only top-level nodes — a scheduled flow whose data ops all live in aloopbody passes the build and is refused at run time #5633 记录(含实测 A/B)。timeRelative描述符跑不通时 authoring 期零诊断 —— 两条 flow lint 一条只看非空、一条只看对象名(#4966 建议 2) #5496 未触碰。验证
新 fixture 先红后绿(方向在跑之前先声明,实际与预测一致:9 条新测试里 7 红 2 绿)。把两个源文件 stash 回
origin/main、只留测试:AssertionError: expected [] to have a length of 1 but got +0—— 纯盲区。where断言失败:where上,不在计数上。这与模板预设的"计数 0 变 1"不同,如实记录。恢复源文件后:
消费半径(该规则族由
authoring-rules.ts供给 cli validate/lint/compile + runtime publish gate):typecheck / build / 字节纪律:
真实 example app A/B(Dogfood 门禁风险面):
app-showcase/app-crm/app-todo共 34 条 flow,改动前后均 0 findings,无新增门禁失败。为证明这个 0 不是"没走到",另做探针:往 showcaseshowcase_batch_reminders的loop_tasksbody 里注入一个 inert condition,确认被抓到且 scope 正确:changeset
.changeset/flow-lint-loop-body-descent.md—"@objectstack/lint": patch。未触碰content/docs/releases/。已知环境红(非本 PR)
ESLint job 当前全仓红,base 侧 #5604(
check:engine-double-contract挂在packages/runtime/src/action-execution-calldata-not-found.test.ts,#5584 引入)。本 PR 未新增假引擎delete(),不涉及 #5197 门禁。若遇 Check Changeset 竞态红见 #5580;merge queue 若出现ENOENT rename ...tgz.downloading签名见 #5517。Generated by Claude Code