Skip to content

fix(lint): flow 规则族下钻 loop body 及所有嵌套区域 (#5383) - #5635

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-5383-flow-lint-loop-descent
Aug 5, 2026
Merged

fix(lint): flow 规则族下钻 loop body 及所有嵌套区域 (#5383)#5635
os-zhuang merged 1 commit into
mainfrom
claude/issue-5383-flow-lint-loop-descent

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #5383

问题

packages/lint 的 flow 反模式规则族从 flow 顶层平铺nodes / edges,从不下钻 ADR-0031 容器(loopconfig.bodyparallelbranches[]try_catchtry/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,消息仍然唯一定位到一个节点:

flow 'campaign_enrollment' · loop 'loop_leads' body · node 'check_not_enrolled' (decision)

顶层图的 scope 是空串,所以顶层 finding 的文案逐字节不变

覆盖到的规则:flow-inert-node-conditionflow-decision-unconditional-branchflow-branch-label-unmatchedflow-default-edge-with-conditionflow-multiple-default-edgesflow-double-brace-interpolationflow-bare-dollar-referenceflow-date-equality-filterflow-phantom-aggregationflow-error-label-not-fault,以及 scanApprovalReviseLoops / scanErrorLabelledEdges / scanBranchRouting 三个 scan 函数。其中 flow-default-edge-with-conditionseverity: 'error'——这条修的正是 issue 点出的严重性不对称:一条会挡构建的规则此前看不见下一层写的矛盾。

两处是承重的,不是顺手

1. 每个区域的 nodes 与 edges 必须成对。 branch-routing 族要把节点和它的出边一起判断,而区域是自洽子图。拍平成一个 nodes 袋 + 一个 edges 袋会两个方向都坏掉:

  • 嵌套 decision 的出边不在顶层 edges 里,于是它读起来"没有出边",outs.length === 0 直接跳过——静默漏判;
  • 节点 id 只在图内唯一、不是 flow 内唯一,所以两个不同区域里同名的节点会被合并出边,凭空造出一个 fan-out 误报。

第二种情况有专门的回归测试钉住(两个 parallel branch 各有一个同名 gate 各带一条 isDefault 出边,拍平实现会误报 flow-multiple-default-edges,成对实现保持安静)。

2. 容器自己的 config 在递归扫描时要剥掉区域槽位。 collectTemplateStrings 递归到字符串叶子,而容器的 config 物理上包含所有后代的 config。这里有个反直觉的事实:改动前 loop body 里的 {{ }} 其实已经被看见了,只是被归属到外层 loop 节点上——正是 #4380validate-flow-template-paths 修过的同一种失败模式(看得见,但判在了错误的节点上)。所以如果只下钻而不剥离,就会变成报两次。剥离后 finding 落到真正携带该字符串的节点上,且仍然只报一次。

stripRegionsflow-walk.ts 导出复用,没有复制第二份——"config 去掉区域"这个视图在包内只有一个定义。

边界

验证

新 fixture 先红后绿(方向在跑之前先声明,实际与预测一致:9 条新测试里 7 红 2 绿)。把两个源文件 stash 回 origin/main、只留测试:

Tests  7 failed | 72 passed (79)
  • 6 条"嵌套可见性"用例:AssertionError: expected [] to have a length of 1 but got +0 —— 纯盲区。
  • double-brace 用例:计数断言(第 985 行)通过,只有 where 断言失败:
    Expected: "flow 'campaign_enrollment' · loop 'loop_leads' body · node 'send_reminder' (notify)"
    Received: "flow 'campaign_enrollment' · node 'loop_leads' (loop)"
    
    即这条规则改动前后计数都是 1,变的是归属——所以对它而言"先红"红在 where 上,不在计数上。这与模板预设的"计数 0 变 1"不同,如实记录。
  • 2 条改动前后都绿、如实说明价值所在:顶层 A/B 孪生用例(非回归钉子)与反拍平对照用例(它防的是错误的实现方式,不是旧代码)。

恢复源文件后:

Test Files  59 passed (59)
Tests  1293 passed | 4 skipped (1297)     # @objectstack/lint 全包

消费半径(该规则族由 authoring-rules.ts 供给 cli validate/lint/compile + runtime publish gate):

@objectstack/metadata-protocol   42 passed (42) / 388 passed
@objectstack/cli                 82 passed (82) / 807 passed

typecheck / build / 字节纪律:

pnpm --filter @objectstack/lint typecheck   → tsc --noEmit 无输出
pnpm --filter @objectstack/lint build       → Build success
node scripts/check-nul-bytes.mjs            → OK (5533 files, no raw ASCII control bytes)
eslint(三个改动文件)                        → 无告警

真实 example app A/B(Dogfood 门禁风险面):app-showcase / app-crm / app-todo 共 34 条 flow,改动前后均 0 findings,无新增门禁失败。为证明这个 0 不是"没走到",另做探针:往 showcase showcase_batch_remindersloop_tasks body 里注入一个 inert condition,确认被抓到且 scope 正确:

[warning] flow-inert-node-condition @ flow 'showcase_batch_reminders' · loop 'loop_tasks' body · node 'send_reminder' (notify)

changeset

.changeset/flow-lint-loop-body-descent.md"@objectstack/lint": patch。未触碰 content/docs/releases/

已知环境红(非本 PR)

ESLint job 当前全仓红,base 侧 #5604check: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

…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.
@vercel

vercel Bot commented Aug 5, 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)
objectstack Ignored Ignored Aug 5, 2026 9:04pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/l labels Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/lint.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/hook-bodies.mdx (via @objectstack/lint)
  • content/docs/permissions/authorization.mdx (via @objectstack/lint)
  • content/docs/releases/v17.mdx (via @objectstack/lint)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review August 5, 2026 21:08
@os-zhuang
os-zhuang enabled auto-merge August 5, 2026 21:08
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit d4edb5d Aug 5, 2026
24 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-5383-flow-lint-loop-descent branch August 5, 2026 21:26
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants