fix(lint): flow 规则能看进 try_catch / loop / parallel 嵌套 region(#4380)

.changeset/flow-nested-region-walk.md

@@ -0,0 +1,77 @@
+---
+"@objectstack/lint": patch
+---
+
+fix(lint): flow rules see into try_catch / loop / parallel regions (#4380)
+
+Every lint rule that inspects flow nodes had hand-written the same one-liner —
+
+```ts
+const nodes = Array.isArray(flow.nodes) ? (flow.nodes as AnyRec[]) : [];
+```
+
+— and every one of them 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 before the fix, the same bad nodes at the top level vs inside a
+`try_catch`:
+
+| rule | severity | flat | nested |
+| :--- | :--- | :--- | :--- |
+| `flow-node-write-unknown-field` | error | 1 | **0** |
+| `flow-update-readonly-field` | error | 1 | **0** |
+| `approval-approver-*` | error/warning | 1 | **0** |
+| `flow-template-unknown-field` (filter position) | error | 1 | **1, as a warning** |
+
+**The last row is the one a reader would not predict.**
+`validate-flow-template-paths` 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") silently degraded to an advisory warning, reported against
+the wrapping `try_catch` instead of the `get_record` that is broken:
+
+```
+FLAT     error  	flow "f" node "get_record"	flows[0].nodes[1]
+NESTED   warning	flow "f" node "try_catch"  	flows[0].nodes[1]
+```
+
+Being visible is not the same as being judged correctly. That is worse than a
+clean miss: a yellow line reads as "checked and merely advisory".
+
+**One shared walk, not five.** `flow-walk.ts` — the flow-side counterpart of the
+existing `page-walk.ts`, and here for the same stated reason: getting the
+traversal right is subtle enough that duplicating it has already produced dead
+rules. `walkFlowNodes(flow, flowPath)` 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 now route
+through it: the two flow write rules, the template-path rule, and the approval
+rule.
+
+Findings now land on the node that is actually wrong, which is the point — a
+path pointing at the container is not actionable in a flow with several regions.
+
+**The double-count trap is handled, not left to each caller.** A container node
+is walked too (it has its own config worth checking — a `loop`'s `collection`, a
+`try_catch`'s `retry`), but its `config` physically contains every descendant,
+so a rule that scans config recursively would report each nested finding twice.
+`WalkedFlowNode.localConfig` is the container's config with region slots
+removed; the recursive scanner uses it, and a test pins that a nested token is
+reported once while the container's own `collection` token still is.
+
+`REGION_SLOTS` is declared as data and pinned against the spec's own
+region-bearing config schemas — derived behaviourally (a slot is one that
+accepts `{nodes: […]}`), not restated — so a fifth construct fails that test
+instead of becoming a fifth silent blind spot. A `MAX_REGION_DEPTH` cap 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 (`showcase_resilient_sync`) 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` with
+`flows[24].nodes[1].config.catch.nodes[0].config.fields.sync_statuss` and the
+region trail `try_catch "Push with retry" › catch › node "Flag Sync Failure"`.

packages/lint/src/flow-walk.test.ts

@@ -0,0 +1,206 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import {
+  LoopConfigSchema,
+  ParallelConfigSchema,
+  TryCatchConfigSchema,
+} from '@objectstack/spec/automation';
+
+import {
+  walkFlowNodes,
+  flowNodeLabel,
+  REGION_SLOTS,
+  REGION_CONFIG_KEYS,
+  MAX_REGION_DEPTH,
+} from './flow-walk.js';
+
+const node = (id: string, extra: Record<string, unknown> = {}) => ({ id, type: 'script', ...extra });
+
+describe('REGION_SLOTS — pinned against the spec, not restated', () => {
+  // The ledger's job is to make a NEW region-bearing construct fail here rather
+  // than become a fifth silent blind spot. Derived behaviourally from the
+  // spec's own config schemas: a region slot is one that accepts `{nodes: […]}`.
+  const REGION_BEARING_CONFIGS = {
+    try_catch: TryCatchConfigSchema,
+    loop: LoopConfigSchema,
+    parallel: ParallelConfigSchema,
+  } as const;
+
+  /** Keys of `schema` that accept a region (or an array of them). */
+  const regionKeysOf = (schema: { safeParse: (v: unknown) => { success: boolean; data?: unknown } }): string[] => {
+    // `label` is required by FlowNodeSchema — a probe node without it fails the
+    // parse and would make every slot look non-region.
+    const region = { nodes: [{ id: 'probe', type: 'script', label: 'Probe' }], edges: [] };
+    const probes: Record<string, unknown> = {
+      // Required siblings so the parse reaches the region keys at all.
+      collection: '{items}',
+      try: region,
+      catch: region,
+      body: region,
+      branches: [{ nodes: region.nodes, edges: [] }, { nodes: region.nodes, edges: [] }],
+    };
+    const parsed = schema.safeParse(probes);
+    if (!parsed.success) return [];
+    const data = parsed.data as Record<string, unknown>;
+    return Object.keys(data).filter((k) => {
+      const v = data[k];
+      if (Array.isArray(v)) return v.every((e) => !!e && typeof e === 'object' && Array.isArray((e as never)['nodes']));
+      return !!v && typeof v === 'object' && Array.isArray((v as Record<string, unknown>).nodes);
+    });
+  };
+
+  it('declares exactly the region slots each construct actually accepts', () => {
+    for (const [type, schema] of Object.entries(REGION_BEARING_CONFIGS)) {
+      expect([...(REGION_SLOTS.get(type) ?? [])].sort(), `region slots for '${type}'`).toEqual(
+        regionKeysOf(schema).sort(),
+      );
+    }
+  });
+
+  it('derives REGION_CONFIG_KEYS from the per-type slots', () => {
+    expect([...REGION_CONFIG_KEYS].sort()).toEqual([...new Set([...REGION_SLOTS.values()].flat())].sort());
+  });
+});
+
+describe('walkFlowNodes', () => {
+  it('yields top-level nodes with a flat path and an empty trail', () => {
+    const walked = walkFlowNodes({ nodes: [node('a'), node('b')] }, 'flows[0]');
+    expect(walked.map((w) => w.path)).toEqual(['flows[0].nodes[0]', 'flows[0].nodes[1]']);
+    expect(walked.every((w) => w.regionTrail === '' && w.depth === 0)).toBe(true);
+  });
+
+  it('reaches try_catch try + catch regions', () => {
+    const flow = {
+      nodes: [
+        {
+          id: 'guard',
+          type: 'try_catch',
+          label: 'Guard',
+          config: {
+            try: { nodes: [node('push')], edges: [] },
+            catch: { nodes: [node('flag')], edges: [] },
+          },
+        },
+      ],
+    };
+    const walked = walkFlowNodes(flow, 'flows[0]');
+    expect(walked.map((w) => w.path)).toEqual([
+      'flows[0].nodes[0]',
+      'flows[0].nodes[0].config.try.nodes[0]',
+      'flows[0].nodes[0].config.catch.nodes[0]',
+    ]);
+    expect(walked[2].regionTrail).toBe('try_catch "Guard" › catch');
+    expect(walked[2].depth).toBe(1);
+  });
+
+  it('reaches a loop body', () => {
+    const flow = {
+      nodes: [{ id: 'each', type: 'loop', config: { collection: '{items}', body: { nodes: [node('inner')], edges: [] } } }],
+    };
+    const walked = walkFlowNodes(flow, 'flows[0]');
+    expect(walked.map((w) => w.path)).toEqual(['flows[0].nodes[0]', 'flows[0].nodes[0].config.body.nodes[0]']);
+    expect(walked[1].regionTrail).toBe('loop "each" › body');
+  });
+
+  it('reaches every parallel branch, named or indexed', () => {
+    const flow = {
+      nodes: [
+        {
+          id: 'fan',
+          type: 'parallel',
+          config: {
+            branches: [
+              { name: 'left', nodes: [node('l')], edges: [] },
+              { nodes: [node('r')], edges: [] },
+            ],
+          },
+        },
+      ],
+    };
+    const walked = walkFlowNodes(flow, 'flows[0]');
+    expect(walked.map((w) => w.path)).toEqual([
+      'flows[0].nodes[0]',
+      'flows[0].nodes[0].config.branches[0].nodes[0]',
+      'flows[0].nodes[0].config.branches[1].nodes[0]',
+    ]);
+    expect(walked[1].regionTrail).toBe('parallel "fan" › branch left');
+    expect(walked[2].regionTrail).toBe('parallel "fan" › branch #1');
+  });
+
+  it('recurses through nested regions and accumulates the trail', () => {
+    const flow = {
+      nodes: [
+        {
+          id: 'outer',
+          type: 'try_catch',
+          config: {
+            try: {
+              nodes: [
+                {
+                  id: 'inner',
+                  type: 'loop',
+                  config: { collection: '{x}', body: { nodes: [node('deep')], edges: [] } },
+                },
+              ],
+              edges: [],
+            },
+          },
+        },
+      ],
+    };
+    const walked = walkFlowNodes(flow, 'flows[0]');
+    const deep = walked.find((w) => w.node.id === 'deep');
+    expect(deep?.path).toBe('flows[0].nodes[0].config.try.nodes[0].config.body.nodes[0]');
+    expect(deep?.regionTrail).toBe('try_catch "outer" › try › loop "inner" › body');
+    expect(deep?.depth).toBe(2);
+  });
+
+  // The trap that makes a recursive config scan double-report.
+  it('localConfig strips region slots but keeps the container’s own config', () => {
+    const flow = {
+      nodes: [
+        {
+          id: 'each',
+          type: 'loop',
+          config: { collection: '{items}', maxIterations: 10, body: { nodes: [node('inner')], edges: [] } },
+        },
+      ],
+    };
+    const [container, inner] = walkFlowNodes(flow, 'flows[0]');
+    expect(Object.keys(container.localConfig ?? {}).sort()).toEqual(['collection', 'maxIterations']);
+    // The raw node is untouched — stripping is a view, not a mutation.
+    expect((container.node.config as Record<string, unknown>).body).toBeDefined();
+    // A non-container node's localConfig is its config, not a copy-with-holes.
+    expect(inner.localConfig).toEqual(inner.node.config ?? undefined);
+  });
+
+  it('leaves localConfig undefined for a node with no config', () => {
+    const [only] = walkFlowNodes({ nodes: [{ id: 'x', type: 'end' }] }, 'flows[0]');
+    expect(only.localConfig).toBeUndefined();
+  });
+
+  it('labels a node by label, then id, then index', () => {
+    expect(flowNodeLabel({ label: 'L', id: 'i' }, 0)).toBe('L');
+    expect(flowNodeLabel({ id: 'i' }, 0)).toBe('i');
+    expect(flowNodeLabel({}, 3)).toBe('#3');
+  });
+
+  it('tolerates missing/!array nodes and non-record entries', () => {
+    expect(walkFlowNodes({}, 'flows[0]')).toEqual([]);
+    expect(walkFlowNodes({ nodes: 'nope' }, 'flows[0]')).toEqual([]);
+    expect(walkFlowNodes({ nodes: [null, 'x', node('ok')] }, 'flows[0]').map((w) => w.node.id)).toEqual(['ok']);
+    expect(walkFlowNodes({ nodes: [{ id: 'c', type: 'try_catch', config: { try: 'nope' } }] }, 'flows[0]')).toHaveLength(1);
+  });
+
+  it('stops at the depth cap rather than recursing without bound', () => {
+    // Build MAX_REGION_DEPTH + 3 levels of try nesting.
+    let deepest: Record<string, unknown> = { id: 'leaf', type: 'script' };
+    for (let i = 0; i < MAX_REGION_DEPTH + 3; i++) {
+      deepest = { id: `t${i}`, type: 'try_catch', config: { try: { nodes: [deepest], edges: [] } } };
+    }
+    const walked = walkFlowNodes({ nodes: [deepest] }, 'flows[0]');
+    expect(walked.length).toBeGreaterThan(0);
+    expect(Math.max(...walked.map((w) => w.depth))).toBeLessThanOrEqual(MAX_REGION_DEPTH);
+  });
+});