fix(automation,objectql): a filter that loses a condition must not run (#3810)

.changeset/flow-filter-collapse-and-write-path-tokens.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/service-automation": minor
+"@objectstack/spec": patch
+"@objectstack/lint": patch
+---
+
+fix(automation,objectql): a filter that loses a condition must not run (#3810)
+
+Three related holes, all of which end in "the query matched rows the author
+excluded".
+
+**1. A flow filter could silently widen to match everything.**
+
+The flow template interpolator expresses "this token did not resolve" as
+`undefined`. In a message that renders as empty text — harmless. In a FILTER it
+removes the condition, and a removed condition matches MORE rows. When it was
+the only condition, `{ owner: '{record.ownr}' }` became `{}`, and `{}` handed to
+`deleteMany` is every row in the table.
+
+So one mistyped field name in a `delete_record` node silently emptied the
+object. Reproduced with all four causes: a typo (`{record.ownr}`), an input the
+run never received, a lookup hop (`{record.account.name}` — the trigger record
+carries a scalar id), and a filter placeholder.
+
+`get_record` / `update_record` / `delete_record` now refuse to execute when
+interpolation erased any authored condition, naming the offending template. The
+guard keys on LOSS, not emptiness: an author who deliberately wrote no filter is
+unaffected, and losing one of two conditions still fails, because widening from
+"my open records" to "all open records" is the same class of bug.
+
+**2. Filter placeholders never reached the engine that resolves them.**
+
+`config.filter` is where two `{…}` dialects meet — the flow template dialect
+(`{record.owner}`) and the filter placeholder dialect (`{current_year_start}`,
+`{current_user_id}`, resolved by `resolveFilterTokens()`). Evaluation order
+picked the winner by accident: the flow interpolator ran first, found no flow
+variable by that name, and erased it.
+
+`interpolateFilter()` hands that position back to the dialect that owns it — a
+whole-string token that no flow variable resolves and that IS a recognised
+placeholder passes through verbatim for the engine to expand. Flow variables
+keep precedence, so a template that works today cannot change meaning.
+
+**3. The engine resolved placeholders on reads but not on writes.**
+
+`resolveFilterTokens()` reached `find`/`findOne`/`count`/`aggregate` only. So
+the SAME filter selected different rows depending on the verb: `find({ owner:
+'{current_user_id}' })` matched the signed-in user's rows, while
+`update`/`delete` compared the literal token text and matched none — a flow that
+previewed with one and acted with the other operated on two different row sets.
+This is the #3106 shape one layer down: the evaluator existed, only some call
+sites reached it.
+
+`update` and `delete` now resolve too, BEFORE the by-id fast path claims a
+scalar `where.id` (otherwise an unresolved `{current_user_id}` would be bound as
+the primary key itself). Caller options are never mutated.

content/docs/references/data/context-tokens.mdx

@@ -29,11 +29,19 @@ the wire (framework#3582): `resolveContextTokens()` in
 
 `resolveFilterTokens()` in `@objectstack/core` on the ObjectQL read
 
-path and the analytics dataset executor for filters that reach the
+AND write paths and the analytics dataset executor, for filters that
 
-database without passing through a renderer. The DRIVER only ever
+reach the database without passing through a renderer. The DRIVER
 
-sees concrete ids, never `\{tokens\}`.
+only ever sees concrete ids, never `\{tokens\}`.
+
+The write verbs matter as much as the read ones (#3810): a filter has
+
+to select the same rows whether `find`, `update` or `delete` consumes
+
+it, or a flow that previews with one and acts with the other operates
+
+on two different row sets.
 
 The server resolver reads `ExecutionContext` — `\{current_user_id\}` is
 

content/docs/references/data/date-macros.mdx

@@ -31,15 +31,23 @@ before the filter is handed to the data source.
 
 - **Server** — `resolveFilterTokens()` in `@objectstack/core`, wired
 
-into the ObjectQL read path (`find`/`findOne`/`count`/`aggregate`)
+into the ObjectQL read AND write paths (`find`/`findOne`/`count`/
 
-and the analytics dataset executor. Filters that reach the database
+`aggregate`/`update`/`delete`) and the analytics dataset executor.
 
-WITHOUT passing through a renderer — dashboard widgets, dataset
+Filters that reach the database WITHOUT passing through a renderer —
 
-definitions, REST query params — need this: before it, the token
+dashboard widgets, dataset definitions, REST query params, flow node
 
-compared as a literal string and matched nothing.
+filters — need this: before it, the token compared as a literal
+
+string and matched nothing. The write verbs are covered for the same
+
+reason (#3810): one filter must select one row set regardless of
+
+which verb consumes it, or a flow's `find` preview and its
+
+`update` act on different rows.
 
 Either way the DRIVER only ever sees ISO date / timestamp strings,
 

packages/lint/src/validate-flow-template-paths.ts

@@ -24,6 +24,14 @@
 // produces output (a blank), nothing is fully broken, and the head object may
 // legitimately come from another installed package (skipped — see below).
 //
+// One position is no longer merely blank at run time: inside a CRUD node's
+// `config.filter`, an unresolved token used to DELETE the condition from the
+// query, which widens it — `delete_record` with its only condition gone matched
+// every row. Since framework#3810 those nodes refuse to execute instead. This
+// rule still earns its place there: catching the typo at build time beats a
+// failed run, and it is the only signal for the other config blocks, where the
+// blank-output behaviour is unchanged.
+//
 // Deliberately conservative to keep false positives near zero:
 // - Only `record.`-prefixed tokens are checked. Other `{var}` tokens address
 // flow variables / node outputs the rule cannot resolve statically.

packages/objectql/src/engine-filter-tokens.test.ts

@@ -53,7 +53,10 @@ const DEAL_SCHEMA = {
 };
 
 function makeDriver() {
- const seen: { findAst?: any; findOneAst?: any; countAst?: any; aggregateAst?: any } = {};
+ const seen: {
+ findAst?: any; findOneAst?: any; countAst?: any; aggregateAst?: any;
+ updateManyAst?: any; deleteManyAst?: any; updateId?: any; deleteId?: any;
+ } = {};
  const driver: any = {
  name: 'memory',
  supports: {},
@@ -63,9 +66,11 @@ function makeDriver() {
  findOne: vi.fn(async (_o: string, ast: any) => { seen.findOneAst = ast; return null; }),
  count: vi.fn(async (_o: string, ast: any) => { seen.countAst = ast; return 0; }),
  aggregate: vi.fn(async (_o: string, ast: any) => { seen.aggregateAst = ast; return []; }),
- create: vi.fn(),
- update: vi.fn(),
- delete: vi.fn(),
+ create: vi.fn(async (_o: string, d: any) => d),
+ update: vi.fn(async (_o: string, id: any, d: any) => { seen.updateId = id; return { id, ...d }; }),
+ updateMany: vi.fn(async (_o: string, ast: any) => { seen.updateManyAst = ast; return { modified: 0 }; }),
+ delete: vi.fn(async (_o: string, id: any) => { seen.deleteId = id; return true; }),
+ deleteMany: vi.fn(async (_o: string, ast: any) => { seen.deleteManyAst = ast; return { deleted: 0 }; }),
  };
  return { driver, seen };
 }
@@ -175,6 +180,79 @@ describe('engine filter placeholders (framework#3582)', () => {
  expect(seen.findAst?.where).toEqual({ title: 'acme {x} deal', owner: 'usr_2' });
  });
 
+ // ── Write path (framework#3810) ────────────────────────────────────────
+ // The evaluator originally reached only find/findOne/count/aggregate, so the
+ // SAME filter selected different rows depending on the verb: `find` matched
+ // the signed-in user's rows while `update`/`delete` compared the literal
+ // token text and matched none. #3106 one layer down — the switch was right,
+ // the call sites were incomplete.
+ describe('write path', () => {
+ it('updateMany: the driver receives the resolved filter, not the token', async () => {
+ const { driver, seen } = makeDriver();
+ const ql = await makeEngine(driver);
+
+ await ql.update('deal', { title: 'x' }, {
+ where: { owner: '{current_user_id}' }, multi: true, context: CTX,
+ } as any);
+
+ expect(seen.updateManyAst?.where).toEqual({ owner: 'usr_1' });
+ });
+
+ it('deleteMany: the driver receives the resolved filter, not the token', async () => {
+ const { driver, seen } = makeDriver();
+ const ql = await makeEngine(driver);
+
+ await ql.delete('deal', {
+ where: { close_date: { $lt: '{current_year_start}' } }, multi: true, context: CTX,
+ } as any);
+
+ expect(seen.deleteManyAst?.where).toEqual({ close_date: { $lt: THIS_YEAR_START } });
+ });
+
+ it('resolves BEFORE the by-id fast path claims a scalar where.id', async () => {
+ // Ordering regression: the token would otherwise be bound as the primary
+ // key itself (`WHERE id = '{current_user_id}'`).
+ const { driver, seen } = makeDriver();
+ const ql = await makeEngine(driver);
+
+ await ql.update('deal', { title: 'x' }, { where: { id: '{current_user_id}' }, context: CTX } as any);
+
+ expect(seen.updateId).toBe('usr_1');
+ });
+
+ it('read and write agree on the same filter', async () => {
+ const { driver, seen } = makeDriver();
+ const ql = await makeEngine(driver);
+ const filter = { owner: '{current_user_id}' };
+
+ await ql.find('deal', { where: filter, context: CTX });
+ await ql.update('deal', { title: 'x' }, { where: filter, multi: true, context: CTX } as any);
+
+ expect(seen.updateManyAst?.where).toEqual(seen.findAst?.where);
+ });
+
+ it('an unknown placeholder throws before anything is written', async () => {
+ const { driver } = makeDriver();
+ const ql = await makeEngine(driver);
+
+ await expect(
+ ql.delete('deal', { where: { owner: '{current_user}' }, multi: true, context: CTX } as any),
+ ).rejects.toThrow(/current_user_id/);
+ expect(driver.deleteMany).not.toHaveBeenCalled();
+ expect(driver.delete).not.toHaveBeenCalled();
+ });
+
+ it('does not mutate the caller options — flow node config is reused', async () => {
+ const { driver } = makeDriver();
+ const ql = await makeEngine(driver);
+ const options: any = { where: { owner: '{current_user_id}' }, multi: true, context: CTX };
+
+ await ql.update('deal', { title: 'x' }, options);
+
+ expect(options.where).toEqual({ owner: '{current_user_id}' });
+ });
+ });
+
  it('does not mutate the caller filter — view metadata is shared across requests', async () => {
  const { driver } = makeDriver();
  const ql = await makeEngine(driver);

packages/objectql/src/engine.ts

@@ -2620,6 +2620,25 @@ export class ObjectQL implements IDataEngine {
  ast.where = resolveFilterTokens(ast.where, filterTokenContextFrom(execCtx));
  }
 
+ /**
+ * The write-path counterpart of {@link resolveWhereTokens}: return `options`
+ * with `where` placeholders expanded (#3810).
+ *
+ * Returns the SAME object when nothing resolved — `resolveFilterTokens`
+ * returns its input by reference on a placeholder-free tree, so the common
+ * path allocates nothing. When something does resolve, a shallow copy is
+ * made rather than assigning through: `options` belongs to the caller, and
+ * writing back would bake one request's user id into a filter object the
+ * caller may reuse (view metadata and flow node config both get reused).
+ */
+ private withResolvedWhere<T extends { where?: unknown; context?: ExecutionContextInput } | undefined>(
+ options: T,
+ ): T {
+ if (!options || options.where == null) return options;
+ const resolved = resolveFilterTokens(options.where, filterTokenContextFrom(options.context));
+ return resolved === options.where ? options : ({ ...options, where: resolved } as T);
+ }
+
  async find(object: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise<any[]> {
  object = this.resolveObjectName(object);
  this.logger.debug('Find operation starting', { object, query });
@@ -3131,7 +3150,20 @@ export class ObjectQL implements IDataEngine {
  this.logger.debug('Update operation starting', { object });
  this.assertWriteAllowed(object, 'update');
  const driver = this.getDriver(object);
-
+
+ // Expand `{filter-placeholder}` values BEFORE the id is extracted (#3810).
+ // The read path resolves them; without the same call here the SAME filter
+ // selected different rows depending on the verb — `find({owner:
+ // '{current_user_id}'})` matched the signed-in user's rows while
+ // `update`/`delete` compared the literal token text and matched none. That
+ // is the #3106 shape one layer down: the evaluator existed, but only some
+ // call sites reached it.
+ //
+ // Ordering matters: a scalar `where.id` becomes the by-id fast path below,
+ // so an unresolved `{current_user_id}` would be bound as the primary key
+ // itself. Resolve first, then extract.
+ options = this.withResolvedWhere(options);
+
  // 1. Extract ID from data or where if it's a single update by ID.
  // Only a SCALAR `where.id` means "update one row by primary key". An
  // operator object ({ $in: [...] }, { $ne: ... }, …) is a multi-row
@@ -3510,6 +3542,10 @@ export class ObjectQL implements IDataEngine {
  this.assertWriteAllowed(object, 'delete');
  const driver = this.getDriver(object);
 
+ // Expand `{filter-placeholder}` values before the id is extracted — same
+ // reasoning as update() above (#3810).
+ options = this.withResolvedWhere(options);
+
  // Extract ID logic mirroring update(): only a SCALAR `where.id` means
  // "delete one row by primary key". An operator object ({ $in: [...] }, …)
  // is a multi-row predicate — treating it as an id would bind the object