From e8c86ca69d9bbabb66a1b0d58de0795491255dfd Mon Sep 17 00:00:00 2001 From: os-dev Date: Thu, 6 Aug 2026 00:32:01 +0000 Subject: [PATCH 1/2] fix(lint): flow-runas-unscoped searches every region for its data-node evidence (#5633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5383 gave the flow anti-pattern family a per-region walk and deliberately left this one rule reading the flow's top-level `nodes` only, because it is the family's only build-GATING member and widening it has its own blast radius. Its data-node search is the rule's evidence that the flow performs a data operation at all, and a data node inside a `loop` body is exactly as unscoped as one at the top level. So a scheduled flow that queried a set, looped it and wrote per item — the standard shape, where the write is almost always nested — passed the build clean and was then refused at run time (#3760). Passing the build and then being unable to run is precisely what promoting this rule to `error` was for. The search now runs across `collectFlowGraphs(flow)` at any depth while the finding stays flow-level: one per flow, `where` = `flow 'x' · runAs`, because `runAs` is a flow property and the region only supplies the evidence. The region is named in the message instead, so the author can find the node. Top-level behaviour is unchanged, bit for bit: the message for a top-level evidence node is byte-identical (no region clause), and a flow with data nodes at both altitudes still cites the top-level one, since `collectFlowGraphs` yields the flow's own graph before it descends. Both pinned by tests. Verified: `os validate` over app-showcase / app-crm / app-todo is line-for-line identical before and after; the docs/skills flow fixtures A/B is identical too. --- .../flow-runas-unscoped-region-descent.md | 56 +++++ packages/lint/src/lint-flow-patterns.test.ts | 200 ++++++++++++++++++ packages/lint/src/lint-flow-patterns.ts | 100 ++++++++- 3 files changed, 350 insertions(+), 6 deletions(-) create mode 100644 .changeset/flow-runas-unscoped-region-descent.md diff --git a/.changeset/flow-runas-unscoped-region-descent.md b/.changeset/flow-runas-unscoped-region-descent.md new file mode 100644 index 0000000000..d9d048d97b --- /dev/null +++ b/.changeset/flow-runas-unscoped-region-descent.md @@ -0,0 +1,56 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): `flow-runas-unscoped` now sees data nodes nested in a `loop` body / `parallel` branch / `try_catch` region (#5633) + +**This widens the coverage of a build-GATING rule.** `flow-runas-unscoped` is +`severity: 'error'`, so a flow it newly catches goes from a green build to a +failed one. That is the correct outcome — those flows cannot run at all — but it +is a real blast radius and the reason this shipped as its own change rather than +riding along with #5383. + +**What was wrong.** #5383 gave the flow anti-pattern family a per-region walk and +deliberately left this one rule reading the flow's **top-level** `nodes` only. Its +data-node search is the rule's evidence that the flow *performs a data operation +at all* — and a data node inside a `loop` body is exactly as unscoped as one at +the top level. So a scheduled flow that queried a set, looped it, and wrote per +item passed `os build` / `os validate` clean and was then **refused at run time**: +since #3760 a user-less run really does refuse the data operation rather than +running it unscoped. Passing the build and then being unable to run is precisely +what promoting this rule to `error` was for, and the shape it was missing — +query, loop, write per item — is *the* standard shape for a scheduled flow, so +the write is almost always the nested node. + +Measured, same flow with only the node's position changed: + +``` +update_record at TOP level -> 1 finding [error] +update_record INSIDE loop body -> 0 findings (now: 1 finding [error]) +``` + +**What changed.** The data-node search runs across `collectFlowGraphs(flow)` — +every ADR-0031 region, at any depth — while the finding itself stays **flow-level** +exactly as before: one per flow, `where` = ``flow 'x' · runAs``, because `runAs` +is a flow property and the region only supplies the evidence. The region is named +in the **message** so you can find the node: + +``` +flow 'nightly_sweep' · runAs: schedule-triggered flow runs as `runAs:'user'`, but a +schedule run has no trigger user — so its data node 'touch' (update_record), in loop +'loop_rows' body, has no identity to scope to and will be REFUSED at run time. +``` + +**Nothing about the top-level case moved.** A flow whose evidence is a top-level +data node produces the byte-identical message it always did (no region clause), +and when a flow has data nodes at both altitudes the top-level one is still the +node cited — `collectFlowGraphs` yields the flow's own graph before it descends. +Both are pinned by tests. + +**If this newly fails your build:** the flow was already broken at run time. Add +`runAs: 'system'` to declare the elevation the sweep needs (a schedule / +time-relative / API run has no user to scope to — there is none). See ADR-0049, +ADR-0073 D5, #1888, #3760. + +The repo's three example apps (`app-showcase`, `app-crm`, `app-todo`) are +unaffected — `os validate` output is line-for-line identical before and after. diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index f90621760a..ee4208d7d9 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -445,6 +445,206 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- expect(lintFlowPatterns(scheduledDataFlow({ flowType: 'screen', startConfig: {} }))).toHaveLength(0); }); }); + + /** + * #5633 — the data-node search descends into every ADR-0031 region. + * + * #5383/#5635 gave this whole rule family the per-region walk and deliberately + * left THIS rule reading the top-level `nodes` only, because widening a + * build-GATING rule is its own change with its own blast radius. That exemption + * is what this block removes. + * + * The evidence and the verdict are at different altitudes here, and keeping + * them apart is the whole design: + * + * - the VERDICT is flow-level. `runAs` is a flow property and the trigger is a + * property of the start node, so a flow is either unscoped or it is not — + * one finding per flow, `where` = `flow 'x' · runAs`, never per region and + * never once per data node. + * - the EVIDENCE is "does this flow touch data at all", and a `loop` body is + * as much part of the flow as its top level. A data node one level down is + * refused by exactly the same `resolveRunAsIdentity` check (#3760) as one at + * the top; the nesting depth is not a property the runtime consults. + * + * The shape missed until now is not a corner but the DEFAULT shape of a + * scheduled data flow — query a set, loop it, write per item — so the write is + * almost always the nested node. Every case below pins the nested finding + * against its top-level twin, and the twin's message is asserted byte-for-byte + * so the widening cannot drift the wording authors already see. + */ + describe('#5633 — descends into nested regions for the data-node evidence', () => { + /** The canonical scheduled sweep: `get_record` at the top, the write in the loop body. */ + const sweepFlow = (opts: { runAs?: 'system' | 'user'; bodyNodeType?: string } = {}) => ({ + flows: [{ + name: 'nightly_sweep', + type: 'schedule', + ...(opts.runAs ? { runAs: opts.runAs } : {}), + nodes: [ + { id: 'start', type: 'start', config: { triggerType: 'schedule', cron: '0 8 * * *' } }, + { + id: 'loop_rows', type: 'loop', + config: { + collection: '{vars.rows}', itemVar: 'row', + body: { + nodes: [{ + id: 'touch', + type: opts.bodyNodeType ?? 'update_record', + config: { objectName: 'thing', recordId: '{row.id}', fields: { seen: true } }, + }], + edges: [], + }, + }, + }, + { id: 'end', type: 'end' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'loop_rows' }, + { id: 'e2', source: 'loop_rows', target: 'end' }, + ], + }], + }); + + it('flags a loop-body write — the shape that passed the build and is refused at run time', () => { + const fnds = lintFlowPatterns(sweepFlow()); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_RUNAS_UNSCOPED); + expect(fnds[0].severity).toBe('error'); + // The verdict stays FLOW-level: `runAs` is a flow property, so the region + // never enters `where` — it is evidence, not location. + expect(fnds[0].where).toBe("flow 'nightly_sweep' · runAs"); + expect(fnds[0].where).not.toContain('loop'); + // …but the message names the region, or the author has to hunt for the node. + expect(fnds[0].message).toContain("its data node 'touch' (update_record), in loop 'loop_rows' body,"); + }); + + it('flags a loop-body delete_record the same way', () => { + const fnds = lintFlowPatterns(sweepFlow({ bodyNodeType: 'delete_record' })); + expect(fnds.map((f) => f.rule)).toEqual([FLOW_RUNAS_UNSCOPED]); + expect(fnds[0].message).toContain("its data node 'touch' (delete_record), in loop 'loop_rows' body,"); + }); + + // The A/B twin. Same flow, same node, moved out of the body — this was + // already flagged before #5633, and its message must not have moved a byte. + it('leaves the TOP-LEVEL twin byte-identical (no region clause, no regression)', () => { + const fnds = lintFlowPatterns({ + flows: [{ + name: 'nightly_sweep', + type: 'schedule', + nodes: [ + { id: 'start', type: 'start', config: { triggerType: 'schedule', cron: '0 8 * * *' } }, + { id: 'touch', type: 'update_record', config: { objectName: 'thing', fields: { seen: true } } }, + ], + edges: [{ id: 'e1', source: 'start', target: 'touch' }], + }], + }); + expect(fnds).toHaveLength(1); + expect(fnds[0].where).toBe("flow 'nightly_sweep' · runAs"); + expect(fnds[0].message).toBe( + "schedule-triggered flow runs as the default `runAs:'user'`, but a schedule run has no trigger " + + "user — so its data node 'touch' (update_record) has no identity to scope to and " + + "will be REFUSED at run time.", + ); + expect(fnds[0].message).not.toContain('in loop'); + }); + + it('descends two levels — a loop inside a loop', () => { + const fnds = lintFlowPatterns({ + flows: [{ + name: 'nightly_sweep', + type: 'schedule', + nodes: [ + { id: 'start', type: 'start', config: { triggerType: 'schedule', cron: '0 8 * * *' } }, + { + id: 'loop_rows', type: 'loop', + config: { + collection: '{vars.rows}', itemVar: 'row', + body: { + nodes: [{ + id: 'loop_children', type: 'loop', + config: { + collection: '{row.children}', itemVar: 'child', + body: { + nodes: [{ id: 'touch', type: 'create_record', config: { objectName: 'thing', fields: { a: 1 } } }], + edges: [], + }, + }, + }], + edges: [], + }, + }, + }, + ], + edges: [{ id: 'e1', source: 'start', target: 'loop_rows' }], + }], + }).filter((f) => f.rule === FLOW_RUNAS_UNSCOPED); + expect(fnds).toHaveLength(1); + expect(fnds[0].message).toContain( + "its data node 'touch' (create_record), in loop 'loop_rows' body → loop 'loop_children' body,", + ); + }); + + // Exactly ONE finding, not one per data node and not one per region: a + // container's config physically contains its body, and the walk visits the + // body in its own right, so a rule that pushed per hit would double-report. + it('reports ONCE for a flow whose regions hold several data nodes', () => { + const fnds = lintFlowPatterns({ + flows: [{ + name: 'nightly_sweep', + type: 'schedule', + nodes: [ + { id: 'start', type: 'start', config: { triggerType: 'schedule', cron: '0 8 * * *' } }, + { + id: 'fan', type: 'parallel', + config: { + branches: [ + { name: 'writes', nodes: [{ id: 'w1', type: 'update_record', config: { objectName: 'a' } }], edges: [] }, + { name: 'deletes', nodes: [{ id: 'w2', type: 'delete_record', config: { objectName: 'b' } }], edges: [] }, + ], + }, + }, + ], + edges: [{ id: 'e1', source: 'start', target: 'fan' }], + }], + }).filter((f) => f.rule === FLOW_RUNAS_UNSCOPED); + expect(fnds).toHaveLength(1); + expect(fnds[0].message).toContain("in parallel 'fan' branch 0,"); + }); + + // The top-level graph comes first out of `collectFlowGraphs`, so a flow with + // data nodes at BOTH altitudes still cites the top-level one — the exact node + // it cited before #5633. + it('prefers the top-level node as evidence when the flow has both', () => { + const stack = sweepFlow() as { flows: Array<{ nodes: unknown[] }> }; + stack.flows[0].nodes.splice(1, 0, { + id: 'query', type: 'get_record', config: { objectName: 'thing', filter: { done: false } }, + }); + const fnds = lintFlowPatterns(stack).filter((f) => f.rule === FLOW_RUNAS_UNSCOPED); + expect(fnds).toHaveLength(1); + expect(fnds[0].message).toContain("its data node 'query' (get_record) has no identity"); + expect(fnds[0].message).not.toContain('in loop'); + }); + + describe('does NOT flag', () => { + it("a loop-body write under runAs:'system' (the correct shape for a sweep)", () => { + expect(lintFlowPatterns(sweepFlow({ runAs: 'system' })) + .filter((f) => f.rule === FLOW_RUNAS_UNSCOPED)).toHaveLength(0); + }); + + it('a loop-body write on a non-user-less trigger (a record-change run carries a user)', () => { + const stack = sweepFlow() as { flows: Array> }; + stack.flows[0].type = 'record_change'; + (stack.flows[0].nodes as Array>)[0] = { + id: 'start', type: 'start', config: { triggerType: 'record-after-update', objectName: 'thing' }, + }; + expect(lintFlowPatterns(stack).filter((f) => f.rule === FLOW_RUNAS_UNSCOPED)).toHaveLength(0); + }); + + it('a loop body holding no data node at all (runAs stays moot — a notify-only sweep)', () => { + expect(lintFlowPatterns(sweepFlow({ bodyNodeType: 'notify' })) + .filter((f) => f.rule === FLOW_RUNAS_UNSCOPED)).toHaveLength(0); + }); + }); + }); }); /** diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index fcb5efba88..b6f20c348c 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -98,10 +98,38 @@ * that actually carries the string, and the count stays 1. * * Deliberately still flow-level, i.e. read off the top-level nodes only: the - * start-node trigger rules (a trigger is a property of the flow, and a region - * has an entry node, not a `start`) and {@link FLOW_RUNAS_UNSCOPED}, whose - * data-node search is left alone here on purpose — widening a build-GATING rule - * is its own change with its own blast radius, filed separately. + * start-node trigger rules. A trigger is a property of the flow, and a region has + * an entry node, not a `start` — there is nothing one level down for them to read. + * + * ## The one rule whose VERDICT is flow-level but whose EVIDENCE is not (#5633) + * + * {@link FLOW_RUNAS_UNSCOPED} was the third case, and it is neither of the two + * above. #5383 left it top-level-only on purpose — it is the family's only + * build-GATING member, so widening it turns green builds red and deserved its own + * change — and #5633 is that change. Its two halves sit at different altitudes: + * + * - the **verdict** is flow-level and stays there. `flow.runAs` is one + * declaration and the trigger is the start node's, so a flow is either + * unscoped or it is not: one finding per flow, `where` = `flow 'x' · runAs`, + * never per region and never once per data node. + * - the **evidence** — "does this flow perform a data operation at all", the + * condition that makes `runAs` matter — is a question about the whole flow. + * A `loop` body is as much part of it as the top level, and the runtime agrees: + * `resolveRunAsIdentity` refuses a nested write for exactly the reason it + * refuses a top-level one (#3760). Nesting depth is not a property it consults. + * + * So {@link findDataNodeAnywhere} searches every graph while the finding stays + * flow-level, and the region is named in the **message** rather than in `where` + * (`its data node 'touch' (update_record), in loop 'loop_rows' body,`): `where` + * says which declaration is wrong, the message says where to find the proof. A + * top-level hit yields the byte-identical message it always did — pinned in the + * tests, since the wording is what every existing author already sees. + * + * What this fixes is not a corner. Query a set, loop it, write per item is *the* + * shape of a scheduled data flow, so the write is almost always the nested node — + * and because this rule gates the build, the shape it was missing built clean and + * then could not run at all. That is precisely what promoting it to `error` + * (#3760) was for. */ import { @@ -221,6 +249,49 @@ const INERT_CONDITION_NODE_TYPES = new Set([ /** Node types that perform a data operation — the ones `flow.runAs` governs (#1888). */ const DATA_NODE_TYPES = new Set(['get_record', 'create_record', 'update_record', 'delete_record']); +/** + * The first data node ANYWHERE in a flow, with the region it was found in — + * {@link FLOW_RUNAS_UNSCOPED}'s evidence that the flow performs a data operation + * at all (#5633). + * + * Three properties of this search are load-bearing: + * + * - **It returns the FIRST hit, not all of them.** The rule's verdict is about + * `flow.runAs`, a single declaration, so the finding is one per flow and the + * node is only cited to point the author at it. Collecting every data node + * would invite a per-node push and turn one wrong declaration into N + * identical build errors. + * - **Top-level first.** {@link collectFlowGraphs} yields the flow's own graph + * before it descends, so a flow with data nodes at both altitudes still cites + * the same node it cited before this widening — the top-level behaviour is + * bit-for-bit unchanged, including which node appears in the message. + * - **No region strip is needed.** A container's `config` physically contains + * its descendants', which is what forces {@link stripRegions} on the + * recursive config scans — but this search reads `node.type` only, and no + * container type (`loop`/`parallel`/`try_catch`) is a data node type. A nested + * write is therefore seen exactly once, in the region graph that owns it, + * never a second time through its enclosing container. + */ +function findDataNodeAnywhere( + nodes: AnyRec[], + edges: AnyRec[], +): { readonly node: AnyRec; readonly scope: string } | null { + // A cast, not a parse — same contract as the main per-graph walk below: the + // walk touches only `type` / `config`, and the guarded arrays are passed so a + // malformed region cannot make this throw (this module never throws). + for (const graph of collectFlowGraphs({ + nodes: nodes as unknown as FlowNodeParsed[], + edges: edges as unknown as FlowEdgeParsed[], + })) { + for (const node of graph.nodes as unknown as AnyRec[]) { + if (DATA_NODE_TYPES.has(typeof node.type === 'string' ? (node.type as string) : '')) { + return { node, scope: graph.scope }; + } + } + } + return null; +} + /** * #5482 — the two node types that carry the `multi` bulk declaration, with the * words their diagnostic uses for what an unbounded one does. @@ -951,17 +1022,34 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { // the same refusal, but whether a given write carries a user is not // knowable at authoring time. That case is caught at run time only // (#3760) — do not try to approximate it here. + // #5633 — the data-node EVIDENCE is searched across every region, while + // the verdict stays flow-level. The two are at different altitudes on + // purpose: `runAs` is a flow property and the trigger is the start + // node's, so "is this flow unscoped" has exactly one answer per flow — + // but "does it touch data at all" is a question about the whole flow, + // and a `loop` body is as much part of it as the top level. The runtime + // agrees: `resolveRunAsIdentity` refuses a nested write for the same + // reason it refuses a top-level one; nesting depth is not a property it + // consults. Left top-level-only by #5383/#5635 because widening a + // build-GATING rule is its own change with its own blast radius — this + // is that change. The shape it was missing is the DEFAULT one for a + // scheduled data flow: query a set, loop it, write per item. const runAs = typeof flow.runAs === 'string' ? flow.runAs : 'user'; const userLessKind = userLessTriggerKind(flow, startCfg); if (userLessKind && runAs !== 'system') { - const dataNode = nodes.find((n) => DATA_NODE_TYPES.has(typeof n.type === 'string' ? (n.type as string) : '')); + const dataNode = findDataNodeAnywhere(nodes, edges); if (dataNode) { const declared = typeof flow.runAs === 'string' ? `\`runAs:'${runAs}'\`` : `the default \`runAs:'user'\``; + // The region is named in the MESSAGE, not in `where`: `where` says which + // declaration is wrong (`flow 'x' · runAs`, unchanged), the message says + // where to look for the node that proves it. A top-level hit adds nothing + // here, so its wording is byte-identical to before (pinned in the tests). + const at = dataNode.scope ? `, in ${dataNode.scope},` : ''; findings.push({ where: `flow '${flowName}' · runAs`, message: `${userLessKind}-triggered flow runs as ${declared}, but a ${userLessKind} run has no trigger ` + - `user — so its data node '${dataNode.id}' (${dataNode.type}) has no identity to scope to and ` + + `user — so its data node '${dataNode.node.id}' (${dataNode.node.type})${at} has no identity to scope to and ` + `will be REFUSED at run time.`, hint: `Declare \`runAs:'system'\` to make the elevation explicit and intended (the run reads/writes ` + From fee594c3bc10c5c5d9d61d0035f7136c2682a4e7 Mon Sep 17 00:00:00 2001 From: os-dev Date: Thu, 6 Aug 2026 00:58:24 +0000 Subject: [PATCH 2/2] test(lint): the #5633 loop fixtures spell the DECLARED item key, and pin it (#5633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new fixtures bound the loop item with `itemVar`, which `LoopConfigSchema` — a `strictObject` — reports as an `unrecognized_key` (the declared key is `iteratorVariable`; `itemVariable` exists only as a diagnostic alias). Nothing here would have gone red for it: region collection reads `config.body`, which is unaffected, so the rule was being proven against a `loop` an author cannot actually write — the #4966 trap one container down. Re-spelled to `iteratorVariable`, added the `label` that `FlowNodeSchema` requires on the body nodes, and pinned the container against the schema in both directions: the fixture parses green, and the near-miss spelling really is rejected so the pin has teeth. Full `safeParse` green rather than merely "no unrecognized keys", because this rule judges a VALUE verdict (`runAs` against the trigger kind) whose evidence must be a node reachable inside an authorable container. Pre-existing fixtures spelling the same key (the #5383 `loopBodyFlow` helper and the #5482 nested cases) are left alone and filed separately — they are not exposed by this change. --- packages/lint/src/lint-flow-patterns.test.ts | 72 ++++++++++++++------ 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index ee4208d7d9..49233f534d 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { TimeRelativeTriggerSchema } from '@objectstack/spec/automation'; +import { TimeRelativeTriggerSchema, LoopConfigSchema } from '@objectstack/spec/automation'; import { lintFlowPatterns, FLOW_TIME_RELATIVE_ANTIPATTERN, @@ -473,7 +473,21 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- * so the widening cannot drift the wording authors already see. */ describe('#5633 — descends into nested regions for the data-node evidence', () => { - /** The canonical scheduled sweep: `get_record` at the top, the write in the loop body. */ + /** The canonical scheduled sweep: the write lives in the loop BODY. */ + const loopConfig = (bodyNodeType = 'update_record') => ({ + collection: '{vars.rows}', + iteratorVariable: 'row', + body: { + nodes: [{ + id: 'touch', + type: bodyNodeType, + label: 'Touch Row', + config: { objectName: 'thing', recordId: '{row.id}', fields: { seen: true } }, + }], + edges: [], + }, + }); + const sweepFlow = (opts: { runAs?: 'system' | 'user'; bodyNodeType?: string } = {}) => ({ flows: [{ name: 'nightly_sweep', @@ -481,20 +495,7 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- ...(opts.runAs ? { runAs: opts.runAs } : {}), nodes: [ { id: 'start', type: 'start', config: { triggerType: 'schedule', cron: '0 8 * * *' } }, - { - id: 'loop_rows', type: 'loop', - config: { - collection: '{vars.rows}', itemVar: 'row', - body: { - nodes: [{ - id: 'touch', - type: opts.bodyNodeType ?? 'update_record', - config: { objectName: 'thing', recordId: '{row.id}', fields: { seen: true } }, - }], - edges: [], - }, - }, - }, + { id: 'loop_rows', type: 'loop', config: loopConfig(opts.bodyNodeType) }, { id: 'end', type: 'end' }, ], edges: [ @@ -504,6 +505,33 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- }], }); + /** + * The container these fixtures nest the write inside must be a shape the + * schema ACCEPTS, or the rule is only proven on metadata that never reaches + * it — the #4966 trap, one container down (see the `TimeRelativeTriggerSchema` + * pin above for the same guard on a trigger descriptor). + * + * This one is not hypothetical: the item-binding key here is + * `iteratorVariable`, and `LoopConfigSchema` is a `strictObject`, so the + * plausible-looking `itemVar` is reported as an `unrecognized_key` rather + * than quietly ignored (#4001). A fixture spelling it would still exercise + * this rule — region collection reads `config.body`, which is unaffected — + * so nothing here would have gone red while the fixture taught a `loop` the + * author cannot actually write. + * + * Full `safeParse` green rather than merely "no unrecognized keys", because + * what this rule judges is a VALUE verdict (`runAs` against the trigger + * kind), and its evidence is a node that must really be reachable inside a + * really-authorable container. + */ + it('pins the loop container against the schema — the fixture must be authorable', () => { + const parsed = LoopConfigSchema.safeParse(loopConfig()); + expect(parsed.success).toBe(true); + // And the near-miss spelling really is rejected, so the pin has teeth. + const near = LoopConfigSchema.safeParse({ ...loopConfig(), iteratorVariable: undefined, itemVar: 'row' }); + expect(near.success).toBe(false); + }); + it('flags a loop-body write — the shape that passed the build and is refused at run time', () => { const fnds = lintFlowPatterns(sweepFlow()); expect(fnds).toHaveLength(1); @@ -557,14 +585,14 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- { id: 'loop_rows', type: 'loop', config: { - collection: '{vars.rows}', itemVar: 'row', + collection: '{vars.rows}', iteratorVariable: 'row', body: { nodes: [{ - id: 'loop_children', type: 'loop', + id: 'loop_children', type: 'loop', label: 'Loop Children', config: { - collection: '{row.children}', itemVar: 'child', + collection: '{row.children}', iteratorVariable: 'child', body: { - nodes: [{ id: 'touch', type: 'create_record', config: { objectName: 'thing', fields: { a: 1 } } }], + nodes: [{ id: 'touch', type: 'create_record', label: 'Create', config: { objectName: 'thing', fields: { a: 1 } } }], edges: [], }, }, @@ -597,8 +625,8 @@ describe('lintFlowPatterns — user-less runAs unscoped (#1888 / ADR-0049 / ADR- id: 'fan', type: 'parallel', config: { branches: [ - { name: 'writes', nodes: [{ id: 'w1', type: 'update_record', config: { objectName: 'a' } }], edges: [] }, - { name: 'deletes', nodes: [{ id: 'w2', type: 'delete_record', config: { objectName: 'b' } }], edges: [] }, + { name: 'writes', nodes: [{ id: 'w1', type: 'update_record', label: 'Write', config: { objectName: 'a' } }], edges: [] }, + { name: 'deletes', nodes: [{ id: 'w2', type: 'delete_record', label: 'Delete', config: { objectName: 'b' } }], edges: [] }, ], }, },