fix(lint): flow 规则能看进 try_catch / loop / parallel 嵌套 region(#4380) - #4388
Merged
Conversation
…4380) Every lint rule that inspects flow nodes had hand-written the same one-liner — `const nodes = Array.isArray(flow.nodes) ? flow.nodes : []` — and every one was therefore blind to the same thing. `FlowRegionSchema` holds a full `nodes: z.array(FlowNodeSchema)`, and four config slots carry one: `try_catch.config.try` / `.catch`, `loop.config.body`, and `parallel.config.branches[].nodes`. Regions nest arbitrarily. Move a node into any of them and the checking stayed behind. Measured, same bad nodes flat vs inside a try_catch: flow-node-write-unknown-field (error) 1 → 0 flow-update-readonly-field (error) 1 → 0 approval-approver-* 1 → 0 flow-template-unknown-field (error) 1 → 1, but as a WARNING The last one is the one a reader would not predict. This rule scans a node's whole config for string leaves, so it still SAW tokens inside a region — but its `filter`-position split only looks at the top level of the node it was handed. A nested filter token lost its position, so the #3810 finding ("this node cannot run — an erased condition WIDENS the query") degraded to an advisory warning, reported against the wrapping try_catch instead of the get_record that is broken. Being visible is not the same as being judged correctly, and that is worse than a clean miss: a yellow line reads as "checked, merely advisory". One shared walk, not five. `flow-walk.ts` is the flow-side counterpart of the existing `page-walk.ts`, here for the same stated reason — getting the traversal right is subtle enough that duplicating it has already produced dead rules. `walkFlowNodes` yields every node with its real config path (`flows[0].nodes[1].config.catch.nodes[0]`), a region breadcrumb for diagnostics (`try_catch "Guard" › catch`), and depth. Four rules route through it. Findings now land on the node that is actually wrong — a path pointing at the container is not actionable in a flow with several regions. The double-count trap is handled once, not left to each caller. A container is walked too (its own config is worth checking — a loop's `collection`, a try_catch's `retry`), but its config physically contains every descendant, so a recursive scanner would report each nested finding twice. `localConfig` is the container's config with region slots removed; a test pins that a nested token is reported once while the container's own token still is. `REGION_SLOTS` is data, pinned against the spec's region-bearing config schemas by behavioural derivation rather than restatement, so a fifth construct fails that test instead of becoming a fifth blind spot. `MAX_REGION_DEPTH` keeps a hand-authored (pre-parse) stack from hanging a lint. Verified end to end: nested now matches flat on every rule, including the restored error severity. app-showcase ships an update_record inside a `catch` branch that had never been checked by anything; it is correct, so validation stays clean, and breaking its field name on purpose now fails `os validate` at `flows[24].nodes[1].config.catch.nodes[0].config.fields.sync_statuss`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Jul 31, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…4401) (#4408) A region is a sub-graph inside `FlowNodeSchema.config`, an open `z.record`. Nothing in the type system says which key on which node type holds one, so every pass that needs to reach a region node has to be told — and within one week three of them were told separately, by two changes that were each correct on their own (#4381 and #4388): mapFlowNodes (ADR-0087 conversions) spec FLOW_REGION_SLOTS validateControlFlow / normalizeControlFlowRegions / collectFlowGraphs spec regionSlotsOf walkFlowNodes (lint flow rules) lint REGION_SLOTS Each pinned its own copy with its own reconciliation test. So every copy was protected from drifting away from the schemas, and nothing would have failed if the copies drifted from each other — while adding a fourth construct meant editing three places, and missing one reproduces exactly the silent blind spot #4347 and #4380 were both filed about. - New `@objectstack/spec/automation` export `FLOW_REGION_SLOTS` (+ the `FLOW_REGION_SLOTS_BY_TYPE` / `FLOW_REGION_CONFIG_KEYS` views) is the only statement of the fact. Import-free module, so `spec/conversions/walk.ts` can read it and stay the pure shape walker it was written as; mapping a slot onto the Zod schema its value parses as stays in `control-flow.zod.ts`. - The three reconciliation tests collapse into `region-slots.test.ts`, keeping the strongest of them (#4388's): it derives each construct's region keys BEHAVIOURALLY, by asking the config schema what it accepts in a region shape, rather than reading names off `.shape`. It also probes every other exported `*ConfigSchema`, so a new region-bearing construct cannot be added without either declaring its slots or failing here. The three walks are deliberately left separate: different inputs (parsed vs raw authored records), different units (a graph, a node, a copy-on-write rewritten tree), and the lint one formats human diagnostic trails from node labels — consumer logic, not protocol (Prime Directive #2). No behaviour change: every existing test passes unchanged. Claude-Session: https://claude.ai/code/session_01HSBbKMdDgHjGpQdrvQUQXj 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.
Closes #4380
问题
packages/lint里每一条走 flow 节点的规则都手写了同一行:于是它们全都对同一件事失明。
FlowRegionSchema持有完整的nodes: z.array(FlowNodeSchema),四个 config 槽位带 region:try_catch.config.try/.catch、loop.config.body、parallel.config.branches[].nodes,而且可以任意嵌套。节点一挪进去,检查就留在了外面。同一组坏节点,顶层 vs
try_catch内部,实测:flow-node-write-unknown-fieldflow-update-readonly-fieldapproval-approver-*flow-template-unknown-field(filter 位置)最后一行才是最麻烦的
validate-flow-template-paths对节点整个config做递归 string-leaf 扫描,所以它碰巧还看得见嵌套节点里的 token——但collectNodeLeaves只在拿到的那个节点的顶层拆filter键。嵌套后位置信息丢失:于是 #3810 那条「条件被抹掉会放宽查询、运行时直接拒绝执行该节点」的 gating 判断,变成一行黄色 advisory,还定位到外层
try_catch而不是真正跑不了的get_record。看得见 ≠ 判断对了,而且这比干脆漏掉更糟:一行黄色读起来像「已经检查过,只是建议性的」。
修法:一次遍历,不是五份
新增
flow-walk.ts——包里已有的page-walk.ts的 flow 侧对应物,理由也照抄它自己写下的那句:这个遍历微妙到「复制它已经造出过一条死规则」。walkFlowNodes(flow, flowPath)产出每个节点及其真实 config 路径(flows[0].nodes[1].config.catch.nodes[0])、用于诊断的 region 面包屑(try_catch "Guard" › catch)和深度。四条规则改走它。Finding 现在落在真正出问题的那个节点上——在有多个 region 的流里,一个指向容器的路径没法据以行动。
重复计数的坑一次性解决,不留给每个调用方
容器节点也会被产出(它自己的 config 值得检查——
loop的collection、try_catch的retry),但它的config物理上包含所有后代,所以递归扫 config 的规则会把每个嵌套 finding 报两遍。WalkedFlowNode.localConfig是剥掉 region 槽位后的容器 config,递归扫描方用它。测试钉住:嵌套 token 只报一次,同时容器自己的collectiontoken 照报。防漂移
REGION_SLOTS写成数据,并对着 spec 自己的 region-bearing config schema 行为式推导(槽位 = 接受{nodes: […]}的键)做 pin,而不是复述——所以第五种构造会让测试失败,而不是变成第五个静默盲区。MAX_REGION_DEPTH防止手写(未 parse)的 stack 把 lint 挂住。验证
error严重级;showcase_resilient_sync的catch分支中有一个update_record,此前从未被任何规则检查过。它字段是对的,所以校验依旧干净;故意把字段名改坏后,os validate现在 exit 1 并报出:@objectstack/lint43 文件 / 689 测试全绿(新增 25 条),typecheck干净。🤖 Generated with Claude Code