From 2b16380267fb594d44ea6c5b75b630f64e963376 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 22:53:45 +0000 Subject: [PATCH 1/2] feat(lint): warn on a `multi: true` delete/update bounded by nothing (#5482) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 #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 #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 (#5383/#5635), because a loop-body sweep is the standard janitor shape. Empty combinator arrays are deliberately out of range: #5322/#5134 already ruled their identities, and deciding them here would be a fourth hand-written copy of a producer-side reduction. --- .changeset/flow-unbounded-bulk-write-lint.md | 70 +++++++ packages/lint/src/index.ts | 1 + packages/lint/src/lint-flow-patterns.test.ts | 209 +++++++++++++++++++ packages/lint/src/lint-flow-patterns.ts | 170 +++++++++++++++ 4 files changed, 450 insertions(+) create mode 100644 .changeset/flow-unbounded-bulk-write-lint.md diff --git a/.changeset/flow-unbounded-bulk-write-lint.md b/.changeset/flow-unbounded-bulk-write-lint.md new file mode 100644 index 0000000000..55b8c9d7df --- /dev/null +++ b/.changeset/flow-unbounded-bulk-write-lint.md @@ -0,0 +1,70 @@ +--- +"@objectstack/lint": patch +--- + +feat(lint): warn when a `multi: true` delete/update is bounded by nothing — the declared whole-object write (#5482) + +A `delete_record` / `update_record` node that declares `multi: true` with no +`filter` (or an empty one) writes the **whole object**: the executor forwards +`where: {}` plus the bulk intent, the data engine classifies that as a legal +`multi` call, and it lands on `driver.deleteMany` / `driver.updateMany` with no +predicate. Every row, every run. + +That path only became authorable with #5393, which gave these nodes a bulk +declaration at all — before it the executor never passed `options.multi`, so the +engine refused every predicate write (`Delete requires an ID or +options.multi=true`) and "empty filter + bulk intent" was not a reachable shape. +Since then it has been reachable and **silent**: `filter` is optional, `multi` is +optional, nothing related the two, and the author's only feedback was the step's +`acted` row count — reported after the rows were gone. The common way to get +here is not malice but an omission: declaring the bulk intent and forgetting the +constraint. + +`os validate` / `os build` now report `flow-multi-write-unfiltered` for it: + +``` +flow 'nightly_purge' · node 'purge' (delete_record) + declares `multi: true` with no `filter` key — this is a WHOLE-OBJECT write, + by declaration: every row of 'lead' is deleted on every run. … +``` + +**A warning, not a gate.** An explicit whole-object purge is something the +platform grants on purpose — the data engine's own dispatch case-set lists "bulk +intent with no predicate at all" as a valid call — so the shape has a legitimate +reading and the run-time path stays open. What was missing was only that the +author hears about it *before* the rows go. For the same reason the fix is not a +schema `refine`: forbidding the shape would delete an intent the engine grants. + +Two ways to satisfy the warning: write the constraint you mean into `filter` +(the bounded-bulk reference shape is app-showcase's `showcase_inquiry_purge`), or +confirm that emptying the object is the intent and keep it. + +**It does not duplicate the #3810 run-time guard, which judges a different +fact.** That guard refuses a node when a condition the author *wrote* +interpolated to nothing (`{record.ownr}` — a typo — leaving `{}`), and it is +deliberately keyed on "a written condition is gone" rather than on "the filter is +empty", because losing one of two conditions also widens the blast radius. So: + +| fact | judged by | when | verdict | +|-------------------------------|--------------------|-----------|---------| +| a written condition vanished | #3810 filter guard | run time | refuse | +| no condition was ever written | this rule | authoring | warn | + +A node with `filter: { owner: '{record.ownr}' }` is silent for this rule (a +condition *is* written) and refused by that one; a node with no `filter` at all +is warned about here and — correctly — allowed there. The diagnostic names the +run-time guard so the two are not mistaken for one check. + +Reported at every nesting depth, which matters because a scheduled sweep whose +per-item work sits in a `loop` body is the standard janitor shape: a finding +inside a region carries the region scope (`flow 'x' · loop 'sweep' body · node +'purge' (delete_record)`), on the traversal #5383/#5635 added to this family. + +Deliberately out of range: an empty **combinator** array (`{ $and: [] }`, +`{ $or: [] }`). #5322/#5134 ruled those and every driver implements the ruling — +empty `$and` is TRUE (so it *is* a whole-object write), empty `$or` is FALSE (so +it matches nothing and must never be warned about) — but telling them apart +requires the boolean-identity reduction, which already exists producer-side in +each driver. A hand-written fourth copy inside a linter is how a scan and a +validator come to answer with two different predicates, so that case is tracked +separately instead. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 1cb0e2230b..55d1da2fde 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -468,6 +468,7 @@ export { FLOW_DEFAULT_EDGE_WITH_CONDITION, FLOW_MULTIPLE_DEFAULT_EDGES, FLOW_INERT_NODE_CONDITION, + FLOW_MULTI_WRITE_UNFILTERED, } from './lint-flow-patterns.js'; export { lintLivenessProperties } from './lint-liveness-properties.js'; diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index 4722ff91a9..6cd390c465 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -20,6 +20,7 @@ import { FLOW_DEFAULT_EDGE_WITH_CONDITION, FLOW_MULTIPLE_DEFAULT_EDGES, FLOW_INERT_NODE_CONDITION, + FLOW_MULTI_WRITE_UNFILTERED, } from './lint-flow-patterns.js'; const CEL = (source: string) => ({ dialect: 'cel', source }); @@ -995,3 +996,211 @@ describe('#5383 — a recursive config scan does not double-report the container expect(fnds[0].where).not.toContain("node 'loop_leads'"); }); }); + +/** + * #5482 — the declared WHOLE-OBJECT write: `multi: true` on a + * `delete_record` / `update_record` with nothing bounding it. + * + * Reachable only since #5393 gave these nodes a bulk declaration: before it the + * executor never passed `options.multi`, the engine refused every predicate + * write, and "empty filter + bulk" was not an authoring surface at all. Measured + * on `origin/main` before this rule existed, all four shapes below — top-level + * delete, empty-object filter, update, and the same node inside a `loop` body — + * returned `[]` from `lintFlowPatterns`. The only feedback an author got was the + * step's `acted` row count, after the rows were gone. + */ + +/** A janitor flow: one bulk write node, scheduled, correctly `runAs: 'system'`. */ +function purgeFlow(nodeType: string, config: unknown) { + return { + flows: [{ + name: 'nightly_purge', + runAs: 'system', + nodes: [ + { id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 3 * * *' } }, + { id: 'purge', type: nodeType, config }, + ], + edges: [{ id: 'e1', source: 'start', target: 'purge' }], + }], + }; +} + +describe('lintFlowPatterns — unbounded bulk write (#5482)', () => { + it('flags a delete_record with `multi: true` and NO filter', () => { + const fnds = lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', multi: true })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_MULTI_WRITE_UNFILTERED); + expect(fnds[0].where).toBe("flow 'nightly_purge' · node 'purge' (delete_record)"); + // Advisory: the engine's dispatch table grants "bulk intent, no predicate" + // on purpose, so the shape is not provably wrong (severity policy at the top + // of lint-flow-patterns.ts). `undefined` is how this family spells warning. + expect(fnds[0].severity).toBeUndefined(); + // Says WHAT it does — the object by name, and that it is every row. + expect(fnds[0].message).toContain('no `filter` key'); + expect(fnds[0].message).toContain('WHOLE-OBJECT write'); + expect(fnds[0].message).toContain("every row of 'lead' is deleted"); + expect(fnds[0].message).toContain('driver.deleteMany'); + // …and that the only run-time feedback arrives too late to help. + expect(fnds[0].message).toMatch(/`acted` row count/); + expect(fnds[0].message).toMatch(/AFTER the rows are gone/); + }); + + it('flags an EMPTY filter the same way, and says which of the two it saw', () => { + const fnds = lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', filter: {}, multi: true })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_MULTI_WRITE_UNFILTERED); + expect(fnds[0].message).toContain('an EMPTY `filter`'); + expect(fnds[0].message).not.toContain('no `filter` key'); + }); + + it('flags an update_record too, in the words of an overwrite', () => { + const fnds = lintFlowPatterns( + purgeFlow('update_record', { objectName: 'lead', fields: { status: 'stale' }, multi: true }), + ); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_MULTI_WRITE_UNFILTERED); + expect(fnds[0].where).toBe("flow 'nightly_purge' · node 'purge' (update_record)"); + expect(fnds[0].message).toContain("every row of 'lead' is overwritten"); + expect(fnds[0].message).toContain('driver.updateMany'); + }); + + it('names the #3810 run-time guard and says the two judge DIFFERENT facts', () => { + const [f] = lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', multi: true })); + // Cross-naming, not duplication: the run-time guard refuses "a condition you + // WROTE is gone"; this rule warns "no condition was ever written". + expect(f.hint).toContain('#3810'); + expect(f.hint).toMatch(/REFUSES this node at run time/); + expect(f.hint).toMatch(/a written condition is gone/); + expect(f.hint).toMatch(/the filter is empty/); + // Both ways out are offered, and the run-time path is explicitly NOT closed. + expect(f.hint).toMatch(/Write the constraint you mean/); + expect(f.hint).toMatch(/warning, not a gate/); + expect(f.hint).toContain('showcase_inquiry_purge'); + }); + + describe('does NOT flag (false-positive guards)', () => { + it('a bulk write BOUNDED by a filter — the showcase purge shape', () => { + expect( + lintFlowPatterns(purgeFlow('delete_record', { + objectName: 'showcase_inquiry', filter: { status: 'closed' }, multi: true, + })), + ).toHaveLength(0); + }); + + it('a filter whose only condition is a TEMPLATE — that is #3810\'s fact, at run time', () => { + // `{record.ownr}` (a typo) interpolates to nothing and the run-time guard + // REFUSES the node. At authoring time the condition is written, so warning + // "nothing bounds this" here would be false — and would put two diagnostics + // on one defect, one of them wrong about what the author did. + expect( + lintFlowPatterns(purgeFlow('delete_record', { + objectName: 'lead', filter: { owner: '{record.ownr}' }, multi: true, + })), + ).toHaveLength(0); + }); + + it('no `multi` at all — the engine refuses that call BY NAME already', () => { + // `Delete requires an ID or options.multi=true`. Nothing silent to warn + // about, and #5482 is scoped to the declared-bulk shape. + expect(lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead' }))).toHaveLength(0); + expect(lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', filter: {} }))).toHaveLength(0); + }); + + it('`multi: false` — the declaration says the opposite', () => { + expect(lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', multi: false }))).toHaveLength(0); + }); + + it('`multi: \'true\'` (a string) — the schema refuses the node, so it cannot run', () => { + // The executor tests `cfg.multi === true` and the schema types the key + // `z.boolean()`; a string is a parse refusal, not declared bulk intent. + expect(lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', multi: 'true' }))).toHaveLength(0); + }); + + it('a node type that carries no `multi` declaration', () => { + // `get_record` does not write and has no bulk intent; `create_record` has + // neither `filter` nor `multi`. A stray key there is the schema's business. + expect(lintFlowPatterns(purgeFlow('get_record', { objectName: 'lead', multi: true }))).toHaveLength(0); + expect(lintFlowPatterns(purgeFlow('create_record', { objectName: 'lead', multi: true }))).toHaveLength(0); + }); + + it('a non-object `filter` — refused by name at execute time, so no run to describe', () => { + expect( + lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', filter: 'status = closed', multi: true })), + ).toHaveLength(0); + }); + + it('an empty COMBINATOR array — deliberately out of range, both directions', () => { + // #5322/#5134 ruled these and every driver implements the ruling: `$and: []` + // is TRUE (this one IS a whole-object write and goes unwarned — filed as a + // follow-up), `$or: []` is FALSE (matches nothing — warning about it would + // be a false alarm). Telling them apart needs the identity REDUCTION, which + // already exists three times producer-side; a fourth hand-written copy in a + // linter is the divergence `engine-delete-dispatch.ts` exists to prevent. + expect( + lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', filter: { $and: [] }, multi: true })), + ).toHaveLength(0); + expect( + lintFlowPatterns(purgeFlow('delete_record', { objectName: 'lead', filter: { $or: [] }, multi: true })), + ).toHaveLength(0); + }); + }); + + /** + * The rule's main habitat. A scheduled sweep whose per-item work sits in a + * `loop` body is the standard shape for a janitor flow, so a rule that only + * saw top-level nodes would miss the case it was written for — the #5383/#5635 + * blind spot, in the exact family that closed it. + */ + describe('inside a nested region (#5383 / #5635)', () => { + it('flags a loop-body sweep, scoped to the region, exactly once', () => { + const fnds = lintFlowPatterns(loopBodyFlow({ + nodes: [ + { id: 'sweep', type: 'delete_record', config: { objectName: 'campaign_member', multi: true } }, + ], + edges: [], + })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_MULTI_WRITE_UNFILTERED); + expect(fnds[0].where).toBe( + "flow 'campaign_enrollment' · loop 'loop_leads' body · node 'sweep' (delete_record)", + ); + // Not attributed to the enclosing container: the `loop`'s own config + // CONTAINS the body, but this rule reads named keys (`multi`, `filter`) off + // each node, and a `loop` declares neither — so there is no second copy. + expect(fnds[0].where).not.toContain("node 'loop_leads'"); + expect(fnds[0].message).toContain("every row of 'campaign_member' is deleted"); + }); + + it('flags an update_record two regions deep', () => { + const fnds = lintFlowPatterns(loopBodyFlow({ + nodes: [{ + id: 'loop_touchpoints', type: 'loop', label: 'Loop Touchpoints', + config: { + collection: '{lead.touchpoints}', itemVar: 'tp', + body: { + nodes: [{ id: 'reset', type: 'update_record', config: { objectName: 'touchpoint', fields: { done: false }, multi: true } }], + edges: [], + }, + }, + }], + edges: [], + })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_MULTI_WRITE_UNFILTERED); + expect(fnds[0].where).toBe( + "flow 'campaign_enrollment' · loop 'loop_leads' body → loop 'loop_touchpoints' body · " + + "node 'reset' (update_record)", + ); + }); + + it('leaves a BOUNDED loop-body sweep alone', () => { + expect(lintFlowPatterns(loopBodyFlow({ + nodes: [{ + id: 'sweep', type: 'delete_record', + config: { objectName: 'campaign_member', filter: { lead_id: '{lead.id}' }, multi: true }, + }], + edges: [], + }))).toHaveLength(0); + }); + }); +}); diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index 91d7331120..3cbc805fd8 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -37,6 +37,15 @@ * do both". Failing a customer's build on a shape we cannot prove wrong is a * worse trade than letting the warning be ignored. * + * #5482 — the declared WHOLE-OBJECT write. A `delete_record`/`update_record` + * with `multi: true` and no filter condition empties (or rewrites) the object on + * every run. It stays a warning under the bar above — the engine's own dispatch + * table grants "bulk intent, no predicate" on purpose, so the shape has a + * legitimate reading — but before this rule the author's only feedback was the + * step's `acted` count, after the rows were gone. See + * {@link scanUnboundedBulkWrites}, including why it is not a second copy of the + * #3810 run-time erased-condition guard. + * * #1874 — time-relative rules via record-change date-EQUALITY. A start-node * trigger condition like `end_date == daysFromNow(60)` on a `record-*` trigger * only fires if the record happens to be written on that exact day; the robust @@ -173,6 +182,18 @@ export const FLOW_DEFAULT_EDGE_WITH_CONDITION = 'flow-default-edge-with-conditio export const FLOW_MULTIPLE_DEFAULT_EDGES = 'flow-multiple-default-edges'; /** #4414 — `config.condition` on a node whose executor never reads it. */ export const FLOW_INERT_NODE_CONDITION = 'flow-inert-node-condition'; +/** + * #5482 — a `delete_record` / `update_record` node that declares `multi: true` + * and bounds it with NOTHING: the whole-object write, by declaration. + * + * Named descriptor-first per the family note on + * `flow-time-relative-descriptor-invalid` in `validate-flow-trigger-readiness.ts` + * (`flow--`, #5496): the descriptor is the + * `multi` bulk declaration on a write node, the verdict is that no predicate + * bounds it. See {@link scanUnboundedBulkWrites} for why this is a warning and + * how it divides labour with the #3810 run-time guard. + */ +export const FLOW_MULTI_WRITE_UNFILTERED = 'flow-multi-write-unfiltered'; /** * Node types that ship in the box. `config.condition` is only ever READ on the @@ -200,6 +221,21 @@ 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']); +/** + * #5482 — the two node types that carry the `multi` bulk declaration, with the + * words their diagnostic uses for what an unbounded one does. + * + * `create_record` has no `filter` and no `multi`; `get_record` has a `filter` + * but no `multi` and does not write. So this is the whole set, and it is spelled + * out rather than derived from {@link DATA_NODE_TYPES} because membership means + * "this executor forwards `config.multi` to the engine as bulk intent", which is + * a claim about the executor, not about being a data node. + */ +const BULK_WRITE_CONSEQUENCE: ReadonlyMap = new Map([ + ['delete_record', { verb: 'deleted', engineCall: 'driver.deleteMany' }], + ['update_record', { verb: 'overwritten', engineCall: 'driver.updateMany' }], +]); + /** * #3863 — an edge LABELLED like an error path but not TYPED as one. * @@ -606,6 +642,134 @@ function scanBranchRouting( } } +/** + * #5482 — is this AUTHORED `filter` provably carrying no condition at all, so + * that the write it is supposed to bound is bounded by nothing? + * + * Exactly two shapes answer `true`, and the narrowness is the point — a warning + * that says "this is the whole object" has to be right about it: + * + * - the key is **absent** (`undefined` / `null`). The executor substitutes `{}` + * (`resolveNodeFilter(cfg.filter ?? {}, …)` in `crud-nodes.ts`). + * - a plain object with **zero own keys** (`{}`), which the executor passes + * through unchanged. + * + * Both arrive at the engine as `where: {}`, which `resolveEngineDeleteDispatch` + * classifies as `multi` (its case-set lists `multi with no predicate at all` as + * legal) and the driver reads as every row — `driver-memory`'s matcher opens + * with `if (!filter || Object.keys(filter).length === 0) return true`. + * + * Everything else is left alone, deliberately: + * + * - **any object with ≥1 key** — including an authored `{token}` that will + * interpolate to nothing. That is the #3810 guard's fact, judged at run time + * against the interpolation result, and this rule must not pre-empt it: at + * authoring time the condition IS written. + * - **an empty combinator array** (`{ $and: [] }`, `{ $or: [] }`). Not because + * the answer is unclear — #5322/#5134 ruled it and every driver implements + * it: empty `$and` is TRUE (so `{ $and: [] }` on a `multi` write IS the whole + * object), empty `$or` is FALSE (so `{ $or: [] }` matches NOTHING and must + * never be warned about), `$not` of an empty group is FALSE. Deciding which + * is which requires the identity REDUCTION, and that reduction already exists + * three times (`reduceFilterNode` in driver-sql, driver-mongodb, and the + * matcher/refusal walk in driver-memory). Hand-writing a fourth copy inside a + * linter is how the scan and the validator come to answer with two different + * predicates — the failure `engine-delete-dispatch.ts` was extracted to + * prevent. Filed separately, to be done from one shared predicate. + * - **a non-object `filter`** (string, array, number). `DeleteRecordConfigSchema` + * /`UpdateRecordConfigSchema` type it `z.record(z.string(), z.unknown())`, so + * the node is refused BY NAME at execute time (`parseNodeConfig`). Warning + * "the object is unbounded" about metadata the schema already rejects would + * describe a run that never happens. + */ +function filterCarriesNoCondition(filter: unknown): boolean { + if (filter === undefined || filter === null) return true; + if (typeof filter !== 'object' || Array.isArray(filter)) return false; + return Object.keys(filter as AnyRec).length === 0; +} + +/** + * #5482 — a `delete_record` / `update_record` that DECLARES `multi: true` and + * bounds it with nothing: the whole-object write. + * + * Reachable only since #5393 gave these nodes a bulk declaration at all. Before + * it, the executor never passed `options.multi`, so the engine refused every + * predicate write (`Delete requires an ID or options.multi=true`) and "empty + * filter + bulk" was not an authoring surface. It is one now, and it is a + * legitimate one: the engine's own dispatch case-set lists `multi with no + * predicate at all` as a valid `multi` call, so an explicit whole-object purge + * is expressible by design (`engine-delete-dispatch.ts`). + * + * Which is why this WARNS and does not gate, and why the fix for #5482 is not a + * spec refine: forbidding the shape would delete an intent the platform grants + * on purpose. What was missing is only that the author hears about it BEFORE the + * rows go — until now the sole feedback was the step's `acted` count, reported + * after the fact. + * + * ## Not a second copy of the #3810 guard + * + * `crud-nodes.ts` refuses a node at run time when a condition the author WROTE + * interpolated to nothing (`{record.ownr}` — a typo — leaving `{}`), and it is + * deliberately keyed on "a condition the author wrote is gone", not on "the + * filter is empty": losing one of two conditions still widens the blast radius, + * and an intentionally empty filter erases nothing, so `crud-filter-guard.test.ts` + * pins that such a filter is still allowed. + * + * So the two judge different facts and neither subsumes the other: + * + * | fact | judged by | when | verdict | + * |---------------------------------------|----------------------|-----------|---------| + * | a written condition vanished | #3810 filter guard | run time | refuse | + * | no condition was ever written | this rule | authoring | warn | + * + * A node with `filter: { owner: '{record.ownr}' }` is silent here (a condition + * IS written) and refused there. A node with no `filter` at all is warned about + * here and — correctly — allowed there. + */ +function scanUnboundedBulkWrites( + at: string, + nodes: AnyRec[], + findings: FlowLintFinding[], +): void { + for (const node of nodes) { + const nodeType = typeof node.type === 'string' ? node.type : ''; + const consequence = BULK_WRITE_CONSEQUENCE.get(nodeType); + if (!consequence) continue; + const cfg = (node.config ?? {}) as AnyRec; + // `=== true` is the executor's own test (`multi: cfg.multi === true`), and + // the schema types the key `z.boolean()` — a `multi: 'true'` is refused by + // the parse, so treating it as declared bulk intent would warn about a node + // that cannot run. + if (cfg.multi !== true) continue; + if (!filterCarriesNoCondition(cfg.filter)) continue; + + const objectName = typeof cfg.objectName === 'string' && cfg.objectName ? cfg.objectName : '(unnamed object)'; + const filterState = cfg.filter === undefined || cfg.filter === null ? 'no `filter` key' : 'an EMPTY `filter`'; + findings.push({ + where: `${at} · node '${String(node.id)}' (${nodeType})`, + message: + `declares \`multi: true\` with ${filterState} — this is a WHOLE-OBJECT write, by declaration: every ` + + `row of '${objectName}' is ${consequence.verb} on every run. The executor forwards \`where: {}\` plus the ` + + `bulk intent, the engine classifies that as a legal \`multi\` call (its dispatch case-set lists ` + + `\`multi with no predicate at all\`), and it lands on \`${consequence.engineCall}\` with no predicate. ` + + `Nothing refuses it at run time, so the only feedback is the step's \`acted\` row count — reported ` + + `AFTER the rows are gone.`, + hint: + `Write the constraint you mean into \`filter\` (e.g. \`{ status: 'closed' }\` — see ` + + `examples/app-showcase \`showcase_inquiry_purge\`, bulk intent bounded by a predicate). If emptying ` + + `'${objectName}' really is the intent, keep it: this is a warning, not a gate, and the run-time path ` + + `stays open. Distinct from the #3810 erased-condition guard, which REFUSES this node at run time when ` + + `a condition you WROTE interpolated to nothing — that guard is keyed on "a written condition is gone" ` + + `and deliberately not on "the filter is empty", which is the fact this rule judges at authoring ` + + `time. (#5482, #5393)`, + // Warning, not `error`: see the severity policy at the top of this file. + // The shape has a legitimate reading the engine grants on purpose, so it is + // not provably wrong — unlike the gating members of this family. + rule: FLOW_MULTI_WRITE_UNFILTERED, + }); + } +} + function scanApprovalReviseLoops( at: string, nodes: AnyRec[], @@ -886,6 +1050,12 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { // unclaimable branch label, an unconditional sibling that runs anyway, // or a self-contradictory / duplicated `isDefault` marker. scanBranchRouting(at, graphNodes, graphEdges, findings); + + // (f) #5482 — a `multi: true` write with nothing bounding it: the declared + // whole-object delete/update. Scanned per graph like the rest, which is + // what puts the loop-body sweep — the standard shape for a scheduled + // purge, and this rule's main habitat — in range (#5383/#5635). + scanUnboundedBulkWrites(at, graphNodes, findings); } } return findings; From f8d0634e035d86a2119a28348d2ef95af65f24b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 23:07:46 +0000 Subject: [PATCH 2/2] refactor(lint): cite the right dispatch authority per write node (#5482) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- packages/lint/src/lint-flow-patterns.test.ts | 8 +++++++ packages/lint/src/lint-flow-patterns.ts | 24 ++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/lint/src/lint-flow-patterns.test.ts b/packages/lint/src/lint-flow-patterns.test.ts index 6cd390c465..f90621760a 100644 --- a/packages/lint/src/lint-flow-patterns.test.ts +++ b/packages/lint/src/lint-flow-patterns.test.ts @@ -1040,6 +1040,10 @@ describe('lintFlowPatterns — unbounded bulk write (#5482)', () => { expect(fnds[0].message).toContain('WHOLE-OBJECT write'); expect(fnds[0].message).toContain("every row of 'lead' is deleted"); expect(fnds[0].message).toContain('driver.deleteMany'); + // The authority it cites is the delete dispatch that is actually extracted + // and case-set-pinned — not a hand-waved "the engine allows it". + expect(fnds[0].message).toContain('delete-dispatch case-set'); + expect(fnds[0].message).toContain('multi with no predicate at all'); // …and that the only run-time feedback arrives too late to help. expect(fnds[0].message).toMatch(/`acted` row count/); expect(fnds[0].message).toMatch(/AFTER the rows are gone/); @@ -1062,6 +1066,10 @@ describe('lintFlowPatterns — unbounded bulk write (#5482)', () => { expect(fnds[0].where).toBe("flow 'nightly_purge' · node 'purge' (update_record)"); expect(fnds[0].message).toContain("every row of 'lead' is overwritten"); expect(fnds[0].message).toContain('driver.updateMany'); + // Update has no extracted dispatch module, so the message cites the branch + // itself rather than borrowing delete's case-set. + expect(fnds[0].message).toContain('bulk branch on `options.multi`'); + expect(fnds[0].message).not.toContain('delete-dispatch case-set'); }); it('names the #3810 run-time guard and says the two judge DIFFERENT facts', () => { diff --git a/packages/lint/src/lint-flow-patterns.ts b/packages/lint/src/lint-flow-patterns.ts index 3cbc805fd8..fcb5efba88 100644 --- a/packages/lint/src/lint-flow-patterns.ts +++ b/packages/lint/src/lint-flow-patterns.ts @@ -231,9 +231,24 @@ const DATA_NODE_TYPES = new Set(['get_record', 'create_record', 'update_record', * "this executor forwards `config.multi` to the engine as bulk intent", which is * a claim about the executor, not about being a data node. */ -const BULK_WRITE_CONSEQUENCE: ReadonlyMap = new Map([ - ['delete_record', { verb: 'deleted', engineCall: 'driver.deleteMany' }], - ['update_record', { verb: 'overwritten', engineCall: 'driver.updateMany' }], +const BULK_WRITE_CONSEQUENCE: ReadonlyMap< + string, + { readonly verb: string; readonly engineCall: string; readonly dispatchNote: string } +> = new Map([ + ['delete_record', { + verb: 'deleted', + engineCall: 'driver.deleteMany', + // The delete dispatch is the one that is EXTRACTED and case-set-pinned + // (`engine-delete-dispatch.ts`), so it can be cited by name. + dispatchNote: "the engine's delete-dispatch case-set lists `multi with no predicate at all` as a legal `multi` call", + }], + ['update_record', { + verb: 'overwritten', + engineCall: 'driver.updateMany', + // Update has no extracted dispatch module, so the branch itself is the + // authority — and its refusal fires only WITHOUT the declaration. + dispatchNote: "the engine takes its bulk branch on `options.multi` alone (`Update requires an ID or options.multi=true` is refused only when the declaration is absent)", + }], ]); /** @@ -750,8 +765,7 @@ function scanUnboundedBulkWrites( message: `declares \`multi: true\` with ${filterState} — this is a WHOLE-OBJECT write, by declaration: every ` + `row of '${objectName}' is ${consequence.verb} on every run. The executor forwards \`where: {}\` plus the ` + - `bulk intent, the engine classifies that as a legal \`multi\` call (its dispatch case-set lists ` + - `\`multi with no predicate at all\`), and it lands on \`${consequence.engineCall}\` with no predicate. ` + + `bulk intent, ${consequence.dispatchNote}, and it lands on \`${consequence.engineCall}\` with no predicate. ` + `Nothing refuses it at run time, so the only feedback is the step's \`acted\` row count — reported ` + `AFTER the rows are gone.`, hint: