From 0d9be4643e24b344d53a963175dd42eb5f2ee556 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Tue, 16 Jun 2026 15:14:45 +0800 Subject: [PATCH 1/2] feat(cli): lint date-equality vs time values in flow query filters (#1874) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the flow anti-pattern lint from trigger CONDITIONS to query FILTERS. A scheduled flow whose get_record filter binds a field directly / via $eq / $in to a time-function value (daysFromNow/today/now/...) silently matches nothing, because a Field.date stores a time component and an exact match against a re-computed timestamp never holds — the bug the templates discrete-tier alerts hit. Range ops ($gte/$lt day windows) are the correct shape and are exempt. New rule flow-date-equality-filter; advisory warning, never fails the build. 10 tests incl. false-positive guards (windows, $or, plain ranges, non-time eq). Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/lint-date-equality-filter.md | 14 ++++ .../cli/src/utils/lint-flow-patterns.test.ts | 60 +++++++++++++++ packages/cli/src/utils/lint-flow-patterns.ts | 75 ++++++++++++++++++- 3 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 .changeset/lint-date-equality-filter.md diff --git a/.changeset/lint-date-equality-filter.md b/.changeset/lint-date-equality-filter.md new file mode 100644 index 0000000000..426c32084e --- /dev/null +++ b/.changeset/lint-date-equality-filter.md @@ -0,0 +1,14 @@ +--- +"@objectstack/cli": minor +--- + +feat(cli): lint date-EQUALITY against time values in flow query filters (#1874) + +The flow anti-pattern lint already flagged a record-change trigger CONDITION +using date equality (`end_date == daysFromNow(60)`). It now also scans +get_record/query node FILTERS for the same footgun: a field bound directly, or +via `$eq` / `$in`, to a time-function value (`daysFromNow`/`today`/`now`/…). +A `Field.date` is stored with a time component, so an exact match against a +re-computed timestamp silently returns nothing — the failure the templates +discrete-tier alerts hit. Range operators (`$gte`/`$lt` day windows) are the +correct shape and are never flagged. Advisory warning; never fails the build. diff --git a/packages/cli/src/utils/lint-flow-patterns.test.ts b/packages/cli/src/utils/lint-flow-patterns.test.ts index 93d54a6754..7aafee1ad5 100644 --- a/packages/cli/src/utils/lint-flow-patterns.test.ts +++ b/packages/cli/src/utils/lint-flow-patterns.test.ts @@ -4,10 +4,24 @@ import { describe, it, expect } from 'vitest'; import { lintFlowPatterns, FLOW_TIME_RELATIVE_ANTIPATTERN, + FLOW_DATE_EQUALITY_FILTER, FLOW_DOUBLE_BRACE_INTERP, FLOW_BARE_DOLLAR_REF, } from './lint-flow-patterns.js'; +const CEL = (source: string) => ({ dialect: 'cel', source }); +/** A scheduled flow with a get_record node carrying `filter`. */ +const filterFlow = (filter: unknown) => ({ + flows: [{ + name: 'expiry_alert', + nodes: [ + { id: 'start', type: 'start', config: { triggerType: 'schedule', schedule: 'cron:0 9 * * *' } }, + { id: 'query', type: 'get_record', config: { objectName: 'contract', filter } }, + ], + edges: [], + }], +}); + const flow = (condition: unknown, triggerType = 'record-after-update') => ({ flows: [{ name: 'renewal_alert', @@ -50,6 +64,52 @@ describe('lintFlowPatterns — time-relative anti-pattern (#1874)', () => { }); }); +describe('lintFlowPatterns — date-equality in query filter (#1874)', () => { + it('flags a field bound directly to a time value (implicit equality)', () => { + const fnds = lintFlowPatterns(filterFlow({ expires_at: CEL('daysFromNow(30)') })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_DATE_EQUALITY_FILTER); + expect(fnds[0].hint).toMatch(/\$gte.*daysFromNow\(N\).*\$lt/); + }); + + it('flags `$in` against time values (the original renewal_alert bug)', () => { + const fnds = lintFlowPatterns(filterFlow({ + status: 'active', + end_date: { $in: [CEL('daysFromNow(60)'), CEL('daysFromNow(30)'), CEL('daysFromNow(7)')] }, + })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_DATE_EQUALITY_FILTER); + }); + + it('flags `$eq` against a time value', () => { + expect(lintFlowPatterns(filterFlow({ d: { $eq: CEL('today()') } }))).toHaveLength(1); + }); + + describe('does NOT flag (false-positive guards)', () => { + it('a one-day window (the correct fix)', () => { + expect(lintFlowPatterns(filterFlow({ + end_date: { $gte: CEL('daysFromNow(7)'), $lt: CEL('daysFromNow(8)') }, + }))).toHaveLength(0); + }); + it('multi-tier windows wrapped in $or', () => { + expect(lintFlowPatterns(filterFlow({ + status: 'active', + $or: [ + { end_date: { $gte: CEL('daysFromNow(7)'), $lt: CEL('daysFromNow(8)') } }, + { end_date: { $gte: CEL('daysFromNow(30)'), $lt: CEL('daysFromNow(31)') } }, + ], + }))).toHaveLength(0); + }); + it('a plain range like `due_date < today()` (overdue query)', () => { + expect(lintFlowPatterns(filterFlow({ status: 'open', due_date: { $lt: CEL('today()') } }))).toHaveLength(0); + }); + it('equality against a non-time value (status, interpolated id)', () => { + expect(lintFlowPatterns(filterFlow({ status: 'active', id: '{record.id}' }))).toHaveLength(0); + expect(lintFlowPatterns(filterFlow({ amount: { $eq: CEL('record.threshold') } }))).toHaveLength(0); + }); + }); +}); + /** A flow with a create_record node carrying `config`. */ const nodeFlow = (config: Record) => ({ flows: [{ diff --git a/packages/cli/src/utils/lint-flow-patterns.ts b/packages/cli/src/utils/lint-flow-patterns.ts index 5899e084f5..278668449f 100644 --- a/packages/cli/src/utils/lint-flow-patterns.ts +++ b/packages/cli/src/utils/lint-flow-patterns.ts @@ -38,6 +38,7 @@ function conditionSource(raw: unknown): string { } const TIME_FNS = 'daysFromNow|daysAgo|today|now|date|datetime'; +const TIME_FN_RE = new RegExp(`\\b(?:${TIME_FNS})\\s*\\(`); // A time function adjacent to an equality operator, either side: // `end_date == daysFromNow(60)` / `today() != record.start` const DATE_EQ = new RegExp( @@ -45,9 +46,74 @@ const DATE_EQ = new RegExp( ); export const FLOW_TIME_RELATIVE_ANTIPATTERN = 'flow-time-relative-antipattern'; +export const FLOW_DATE_EQUALITY_FILTER = 'flow-date-equality-filter'; export const FLOW_DOUBLE_BRACE_INTERP = 'flow-double-brace-interpolation'; export const FLOW_BARE_DOLLAR_REF = 'flow-bare-dollar-reference'; +/** If `v` is a CEL expression whose source calls a time function, return that source. */ +function celTimeSource(v: unknown): string | null { + if (v && typeof v === 'object' && (v as AnyRec).dialect === 'cel') { + const src = (v as AnyRec).source; + if (typeof src === 'string' && TIME_FN_RE.test(src)) return src; + } + return null; +} + +/** Range operators — the building block of the CORRECT time-window pattern, never flagged. */ +const RANGE_OPS = new Set(['$gte', '$gt', '$lte', '$lt', '$ne']); + +/** + * Walk a get_record/query `filter` for the date-EQUALITY footgun: a field bound + * directly (`field: daysFromNow(N)`) or via `$eq` / `$in` to a time-function value. + * A `Field.date` is stored with a time component, so two independently-computed + * timestamps never compare equal — the query silently returns nothing (#1928 / + * templates #1874). Range operators (`$gte`/`$lt` day windows) are the correct + * shape and are never flagged. + */ +function scanFilterForDateEquality( + filter: unknown, + where: string, + findings: FlowLintFinding[], +): void { + if (!filter || typeof filter !== 'object' || Array.isArray(filter)) return; + for (const [key, val] of Object.entries(filter as AnyRec)) { + if (key === '$or' || key === '$and') { + if (Array.isArray(val)) for (const sub of val) scanFilterForDateEquality(sub, where, findings); + continue; + } + // `key` is a field name; `val` is its constraint. + const direct = celTimeSource(val); // `field: daysFromNow(N)` → implicit equality + let hit: { op: string; src: string } | null = direct ? { op: '==', src: direct } : null; + if (!hit && val && typeof val === 'object' && (val as AnyRec).dialect !== 'cel') { + for (const [op, operand] of Object.entries(val as AnyRec)) { + if (RANGE_OPS.has(op)) continue; // correct pattern — leave it + if (op === '$eq') { + const s = celTimeSource(operand); + if (s) { hit = { op: '$eq', src: s }; break; } + } else if (op === '$in' && Array.isArray(operand)) { + for (const item of operand) { + const s = celTimeSource(item); + if (s) { hit = { op: '$in', src: s }; break; } + } + if (hit) break; + } + } + } + if (hit) { + findings.push({ + where, + message: + `filter matches \`${key}\` by ${hit.op} against a time value (\`${hit.src}\`) — a date field carries a ` + + `time component, so exact equality against \`${hit.src}\` (re-computed each run) silently matches nothing.`, + hint: + `Use a one-day window instead: \`${key}: { $gte: daysFromNow(N), $lt: daysFromNow(N+1) }\` ` + + `(wrap multiple tiers in \`$or\`). The abutting windows tile the timeline so each row matches exactly once. (#1874)`, + rule: FLOW_DATE_EQUALITY_FILTER, + }); + } + } +} + // Flow node VALUES interpolate with SINGLE braces (`{var}` / `{rec.field}` / // `{$User.Id}`). Two wrong-syntax mistakes AI/human authors carry over from the // *formula* template dialect (`{{ path }}`) or other platforms: @@ -106,9 +172,16 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { // node values use SINGLE braces; double-brace `{{ }}` and bare `$ref.x` // are carried over from the formula template dialect / other platforms. for (const node of nodes) { + const nodeWhere = `flow '${flowName}' · node '${node.id}' (${node.type})`; + + // (a2) #1874 — date-EQUALITY (`==`/`$eq`/`$in`) against a time value in a + // query filter. A scheduled flow that filters this way silently matches + // nothing; the robust shape is a `$gte`/`$lt` day window. + const cfg = (node.config ?? {}) as AnyRec; + if (cfg.filter) scanFilterForDateEquality(cfg.filter, `${nodeWhere} filter`, findings); + const strings: string[] = []; collectTemplateStrings(node.config, undefined, strings); - const nodeWhere = `flow '${flowName}' · node '${node.id}' (${node.type})`; for (const str of strings) { if (DOUBLE_BRACE.test(str)) { findings.push({ From a02bc958e790a363f67205dc5c6ebdb691801410 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Tue, 16 Jun 2026 15:18:09 +0800 Subject: [PATCH 2/2] feat(cli): add phantom-aggregation flow lint (#1870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A node-config key naming a capability the automation engine lacks (aggregations/aggregate/groupBy/rollup/having) is silently ignored at runtime — the node runs and computes nothing (templates publication_rollup). Flag it and point the author to the data-layer equivalent: Field.summary for a cross-object rollup, Field.formula for a per-record computed value. New rule flow-phantom-aggregation; advisory warning. 3 tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/lint-date-equality-filter.md | 21 +++++++------ .../cli/src/utils/lint-flow-patterns.test.ts | 31 +++++++++++++++++++ packages/cli/src/utils/lint-flow-patterns.ts | 27 ++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/.changeset/lint-date-equality-filter.md b/.changeset/lint-date-equality-filter.md index 426c32084e..878e6026eb 100644 --- a/.changeset/lint-date-equality-filter.md +++ b/.changeset/lint-date-equality-filter.md @@ -2,13 +2,16 @@ "@objectstack/cli": minor --- -feat(cli): lint date-EQUALITY against time values in flow query filters (#1874) +feat(cli): two new flow authoring anti-pattern lints — date-equality filters (#1874) and phantom aggregation (#1870) -The flow anti-pattern lint already flagged a record-change trigger CONDITION -using date equality (`end_date == daysFromNow(60)`). It now also scans -get_record/query node FILTERS for the same footgun: a field bound directly, or -via `$eq` / `$in`, to a time-function value (`daysFromNow`/`today`/`now`/…). -A `Field.date` is stored with a time component, so an exact match against a -re-computed timestamp silently returns nothing — the failure the templates -discrete-tier alerts hit. Range operators (`$gte`/`$lt` day windows) are the -correct shape and are never flagged. Advisory warning; never fails the build. +Extends the build-time flow anti-pattern lint (advisory warnings, never fail the build): + +- **flow-date-equality-filter (#1874)**: a get_record/query filter that binds a + field directly, or via `$eq`/`$in`, to a time-function value + (`daysFromNow`/`today`/`now`/…). A `Field.date` stores a time component, so an + exact match against a re-computed timestamp silently returns nothing. Range + operators (`$gte`/`$lt` day windows) are the correct shape and are exempt. +- **flow-phantom-aggregation (#1870)**: a node config key naming a capability the + automation engine does not have (`aggregations`/`aggregate`/`groupBy`/`rollup`/ + `having`). There is no aggregate node, so the key is silently ignored and the + node computes nothing. Points the author to `Field.summary` / `Field.formula`. diff --git a/packages/cli/src/utils/lint-flow-patterns.test.ts b/packages/cli/src/utils/lint-flow-patterns.test.ts index 7aafee1ad5..63cae54fb4 100644 --- a/packages/cli/src/utils/lint-flow-patterns.test.ts +++ b/packages/cli/src/utils/lint-flow-patterns.test.ts @@ -5,6 +5,7 @@ import { lintFlowPatterns, FLOW_TIME_RELATIVE_ANTIPATTERN, FLOW_DATE_EQUALITY_FILTER, + FLOW_PHANTOM_AGGREGATION, FLOW_DOUBLE_BRACE_INTERP, FLOW_BARE_DOLLAR_REF, } from './lint-flow-patterns.js'; @@ -110,6 +111,36 @@ describe('lintFlowPatterns — date-equality in query filter (#1874)', () => { }); }); +describe('lintFlowPatterns — phantom aggregation capability (#1870)', () => { + const scriptNode = (config: Record) => ({ + flows: [{ + name: 'rollup', + nodes: [ + { id: 'start', type: 'start', config: {} }, + { id: 'sum', type: 'script', config }, + ], + edges: [], + }], + }); + + it('flags `aggregations` on a script node (publication_rollup bug)', () => { + const fnds = lintFlowPatterns(scriptNode({ aggregations: { total: { sum: 'amount' } } })); + expect(fnds).toHaveLength(1); + expect(fnds[0].rule).toBe(FLOW_PHANTOM_AGGREGATION); + expect(fnds[0].hint).toMatch(/Field\.summary/); + }); + + it('flags groupBy / rollup / aggregate / having too', () => { + for (const key of ['groupBy', 'rollup', 'aggregate', 'having']) { + expect(lintFlowPatterns(scriptNode({ [key]: {} })).map((f) => f.rule)).toContain(FLOW_PHANTOM_AGGREGATION); + } + }); + + it('does NOT flag an ordinary script/function node', () => { + expect(lintFlowPatterns(scriptNode({ function: 'helpdesk.triage', inputs: { x: 1 } }))).toHaveLength(0); + }); +}); + /** A flow with a create_record node carrying `config`. */ const nodeFlow = (config: Record) => ({ flows: [{ diff --git a/packages/cli/src/utils/lint-flow-patterns.ts b/packages/cli/src/utils/lint-flow-patterns.ts index 278668449f..ea0256556b 100644 --- a/packages/cli/src/utils/lint-flow-patterns.ts +++ b/packages/cli/src/utils/lint-flow-patterns.ts @@ -47,9 +47,18 @@ const DATE_EQ = new RegExp( export const FLOW_TIME_RELATIVE_ANTIPATTERN = 'flow-time-relative-antipattern'; export const FLOW_DATE_EQUALITY_FILTER = 'flow-date-equality-filter'; +export const FLOW_PHANTOM_AGGREGATION = 'flow-phantom-aggregation'; export const FLOW_DOUBLE_BRACE_INTERP = 'flow-double-brace-interpolation'; export const FLOW_BARE_DOLLAR_REF = 'flow-bare-dollar-reference'; +/** + * Node-config keys that name a capability the automation engine does NOT have. + * There is no aggregate node, so a `script`/`loop`/… node carrying these keys is + * silently ignored — the node runs and computes nothing (templates #1870, + * `publication_rollup`). Aggregation belongs in the data layer, not a flow. + */ +const PHANTOM_AGG_KEYS = new Set(['aggregations', 'aggregate', 'groupBy', 'rollup', 'having']); + /** If `v` is a CEL expression whose source calls a time function, return that source. */ function celTimeSource(v: unknown): string | null { if (v && typeof v === 'object' && (v as AnyRec).dialect === 'cel') { @@ -180,6 +189,24 @@ export function lintFlowPatterns(stack: AnyRec): FlowLintFinding[] { const cfg = (node.config ?? {}) as AnyRec; if (cfg.filter) scanFilterForDateEquality(cfg.filter, `${nodeWhere} filter`, findings); + // (a3) #1870 — a node-config key naming a non-existent capability (there is + // no aggregate node) is silently ignored at runtime, so the node + // computes nothing. Point the author at the data-layer equivalent. + for (const key of Object.keys(cfg)) { + if (PHANTOM_AGG_KEYS.has(key)) { + findings.push({ + where: nodeWhere, + message: + `node config has \`${key}\` — the automation engine has no aggregate node, so \`${key}\` is ` + + `silently ignored and this node computes nothing at runtime.`, + hint: + `Aggregation belongs in the data layer: use \`Field.summary\` for a cross-object rollup ` + + `(sum/count of children), or \`Field.formula\` for a per-record computed value. (#1870)`, + rule: FLOW_PHANTOM_AGGREGATION, + }); + } + } + const strings: string[] = []; collectTemplateStrings(node.config, undefined, strings); for (const str of strings) {