refactor(spec,lint): one declaration of where ADR-0031 regions live (#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>

Author: os-zhuang

.changeset/region-slots-single-declaration.md

@@ -0,0 +1,44 @@
+---
+'@objectstack/spec': patch
+'@objectstack/lint': patch
+---
+
+One declaration of where ADR-0031 regions live (#4401).
+
+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:
+
+| pass | package | table it carried |
+|------|---------|------------------|
+| `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` (plus the
+  `FLOW_REGION_SLOTS_BY_TYPE` / `FLOW_REGION_CONFIG_KEYS` views) is now the only statement
+  of the fact. It lives in an **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`, which is schema business.
+- The three reconciliation tests collapse into one, `region-slots.test.ts`, keeping the
+  strongest of them: it derives each construct's region keys **behaviourally**, by asking
+  the config schema what it actually 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. They take different inputs (parsed
+`FlowNodeParsed` vs raw authored records), yield 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). Merging them would trade a
+duplicated four-line table for a walker that serves nobody well. Only the fact they all
+need is shared.
+
+No behaviour change: every existing test passes unchanged, which is the point of the
+exercise.

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

@@ -2,9 +2,8 @@
 
 import { describe, it, expect } from 'vitest';
 import {
-  LoopConfigSchema,
-  ParallelConfigSchema,
-  TryCatchConfigSchema,
+  FLOW_REGION_SLOTS,
+  FLOW_REGION_CONFIG_KEYS,
 } from '@objectstack/spec/automation';
 
 import {
@@ -17,49 +16,21 @@ import {
 
 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(),
-      );
-    }
+/**
+ * The behavioural probe that used to live here — deriving each construct's
+ * region keys from its own config schema — moved to `region-slots.test.ts` in
+ * the spec, next to the single declaration it now reconciles (#4401). This side
+ * only has to prove the projection is faithful; a renamed or added slot fails
+ * over there, once, for all three walks.
+ */
+describe('REGION_SLOTS — projected from the spec, not restated', () => {
+  it('carries every declared slot, grouped by container type', () => {
+    const projected = [...REGION_SLOTS].flatMap(([type, keys]) => keys.map(key => `${type}.${key}`)).sort();
+    expect(projected).toEqual(FLOW_REGION_SLOTS.map(s => `${s.nodeType}.${s.key}`).sort());
   });
 
-  it('derives REGION_CONFIG_KEYS from the per-type slots', () => {
-    expect([...REGION_CONFIG_KEYS].sort()).toEqual([...new Set([...REGION_SLOTS.values()].flat())].sort());
+  it('shares the flat config-key set by reference', () => {
+    expect(REGION_CONFIG_KEYS).toBe(FLOW_REGION_CONFIG_KEYS);
   });
 });
 

packages/lint/src/flow-walk.ts

@@ -48,6 +48,8 @@
  * read named keys (`config.fields`, `config.objectName`) can use either.
  */
 
+import { FLOW_REGION_SLOTS_BY_TYPE, FLOW_REGION_CONFIG_KEYS } from '@objectstack/spec/automation';
+
 export type AnyRec = Record<string, unknown>;
 
 function isRec(v: unknown): v is AnyRec {
@@ -59,24 +61,28 @@ function strName(v: unknown): string | undefined {
 }
 
 /**
- * Config keys that hold a nested region, by owning node type. Declared as data
- * so a new construct is one entry here rather than a fifth silent blind spot,
- * and pinned against the spec's own region-bearing schemas by
- * `flow-walk.test.ts`.
+ * Config keys that hold a nested region, by owning node type.
+ *
+ * Projected from `@objectstack/spec/automation` rather than declared here
+ * (#4401). This used to be a local copy with its own reconciliation test — as
+ * did the ADR-0087 conversion walk's copy and the spec-side control-flow walk's.
+ * Three tables, three tests each pinning its own copy, and nothing that would
+ * fail if they drifted from ONE ANOTHER: every copy was individually protected
+ * and the set was not. A fourth construct is now a single entry in
+ * `spec/src/automation/region-slots.ts`.
+ *
+ * The **walk** below stays here. It takes raw authored records (not
+ * `FlowNodeParsed`), and it yields per-node diagnostic paths and label trails —
+ * formatting that is lint's business, not the protocol's (Prime Directive #2).
+ * Only the table is shared; the three traversals stay separate because they walk
+ * different units for different consumers.
  */
-export const REGION_SLOTS: ReadonlyMap<string, readonly string[]> = new Map([
-  ['try_catch', ['try', 'catch']],
-  ['loop', ['body']],
-  // `parallel` is the odd one: `config.branches[]` is an ARRAY of regions, not
-  // a region. Handled separately in the walk; named here so the key set stays
-  // one list.
-  ['parallel', ['branches']],
-]);
+export const REGION_SLOTS: ReadonlyMap<string, readonly string[]> = new Map(
+  [...FLOW_REGION_SLOTS_BY_TYPE].map(([type, slots]) => [type, slots.map(s => s.key)]),
+);
 
 /** Every config key that may hold region nodes, across all node types. */
-export const REGION_CONFIG_KEYS: ReadonlySet<string> = new Set(
-  [...REGION_SLOTS.values()].flat(),
-);
+export const REGION_CONFIG_KEYS: ReadonlySet<string> = FLOW_REGION_CONFIG_KEYS;
 
 /**
  * Depth cap. Regions are a tree in parsed metadata, so this is not a cycle

packages/spec/api-surface.json

@@ -2141,6 +2141,9 @@
     "ExecutionStepSkipReasonSchema (const)",
     "FLOW_BUILTIN_NODE_TYPES (const)",
     "FLOW_NODE_EXPRESSION_PATHS (const)",
+    "FLOW_REGION_CONFIG_KEYS (const)",
+    "FLOW_REGION_SLOTS (const)",
+    "FLOW_REGION_SLOTS_BY_TYPE (const)",
     "FLOW_STRUCTURAL_NODE_TYPES (const)",
     "Flow (type)",
     "FlowEdge (type)",
@@ -2155,8 +2158,10 @@
     "FlowNodeSchema (const)",
     "FlowParsed (type)",
     "FlowRegion (type)",
+    "FlowRegionArity (type)",
     "FlowRegionParsed (type)",
     "FlowRegionSchema (const)",
+    "FlowRegionSlotDecl (interface)",
     "FlowRunGateSummary (type)",
     "FlowRunGateSummarySchema (const)",
     "FlowRunNodeSummary (type)",

packages/spec/src/automation/control-flow.zod.ts

@@ -49,6 +49,7 @@ import { z } from 'zod';
 import { lazySchema } from '../shared/lazy-schema';
 import { FlowNodeSchema, FlowEdgeSchema } from './flow.zod';
 import type { FlowNodeParsed, FlowEdgeParsed } from './flow.zod';
+import { FLOW_REGION_SLOTS_BY_TYPE } from './region-slots';
 
 // ─── Canonical construct type ids ────────────────────────────────────
 
@@ -322,46 +323,51 @@ interface RegionSlot {
 }
 
 /**
- * The region slots one node carries — the single place that knows where each
- * ADR-0031 container keeps its nested graph(s). Three passes read it
- * ({@link validateControlFlow}, {@link normalizeControlFlowRegions},
- * {@link collectFlowGraphs}), which is exactly why it is one function: a fourth
- * container construct is added here once, not in three walks that drift.
+ * Resolve one node's region slots against the shared declaration
+ * ({@link FLOW_REGION_SLOTS_BY_TYPE}, #4401) — binding each declared slot to
+ * the value it holds, the Zod schema that value parses as, and a diagnostic
+ * label.
+ *
+ * The three passes in this module read it ({@link validateControlFlow},
+ * {@link normalizeControlFlowRegions}, {@link collectFlowGraphs}). WHERE the
+ * slots are is no longer stated here — that moved to `region-slots.ts` so the
+ * conversion walk and the lint walk read the same list. What stays here is the
+ * schema half, which is this module's business.
  *
  * Emits a slot for a declared key even when its value is not region-shaped —
  * `validateControlFlow` needs to reject that, not skip it.
  */
 function regionSlotsOf(node: FlowNodeParsed): RegionSlot[] {
   const cfg = node.config as Record<string, unknown> | undefined;
   if (!cfg) return [];
-
-  if (node.type === LOOP_NODE_TYPE) {
-    return cfg.body == null
-      ? []
-      : [{ raw: cfg.body, key: 'body', label: `loop '${node.id}' body`, schema: FlowRegionSchema }];
-  }
-  if (node.type === PARALLEL_NODE_TYPE && Array.isArray(cfg.branches)) {
-    return cfg.branches.map((raw, index) => ({
-      raw,
-      key: 'branches',
-      index,
-      label: `parallel '${node.id}' branch ${index}`,
-      // A branch also carries an optional `name`, which the plain region schema
-      // (a non-strict `z.object`) would strip.
-      schema: ParallelBranchSchema,
-    }));
-  }
-  if (node.type === TRY_CATCH_NODE_TYPE) {
-    const slots: RegionSlot[] = [];
-    if (cfg.try != null) {
-      slots.push({ raw: cfg.try, key: 'try', label: `try_catch '${node.id}' try`, schema: FlowRegionSchema });
+  const declared = FLOW_REGION_SLOTS_BY_TYPE.get(node.type);
+  if (!declared) return [];
+
+  const slots: RegionSlot[] = [];
+  for (const { key, arity } of declared) {
+    const value = cfg[key];
+    if (arity === 'many') {
+      if (!Array.isArray(value)) continue;
+      value.forEach((raw, index) => slots.push({
+        raw,
+        key,
+        index,
+        label: `${node.type} '${node.id}' ${singularize(key)} ${index}`,
+        // A branch also carries an optional `name`, which the plain region
+        // schema (a non-strict `z.object`) would strip.
+        schema: ParallelBranchSchema,
+      }));
+      continue;
     }
-    if (cfg.catch != null) {
-      slots.push({ raw: cfg.catch, key: 'catch', label: `try_catch '${node.id}' catch`, schema: FlowRegionSchema });
-    }
-    return slots;
+    if (value == null) continue;
+    slots.push({ raw: value, key, label: `${node.type} '${node.id}' ${key}`, schema: FlowRegionSchema });
   }
-  return [];
+  return slots;
+}
+
+/** `branches` → `branch`, for the per-item label of a `many` slot. */
+function singularize(key: string): string {
+  return key.endsWith('es') ? key.slice(0, -2) : key.replace(/s$/, '');
 }
 
 /**

packages/spec/src/automation/index.ts

@@ -2,6 +2,7 @@
 
 
 export * from './flow.zod';
+export * from './region-slots';
 export * from './control-flow.zod';
 export * from './io-node-config.zod';
 export * from './builtin-node-config.zod';