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

Three exits from one failure mode: 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; in a FILTER it removes the condition, and a removed
condition matches MORE rows. When it was the only one, `{ 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, an input the run never received, a lookup
hop, and a filter placeholder. The CRUD nodes now refuse to execute when
interpolation erased any authored condition, naming the template. The guard keys on
LOSS, not emptiness, so a deliberately empty filter still runs and losing one of two
conditions still fails.

2. FILTER PLACEHOLDERS NEVER REACHED THE ENGINE THAT RESOLVES THEM. `config.filter`
is where the flow template dialect and the filter placeholder dialect meet, and
evaluation order picked the winner by accident. `interpolateFilter()` hands that
position back to the dialect that owns it: a whole-string token no flow variable
resolves, that IS a recognised placeholder, passes through verbatim for the engine.
Flow variables keep precedence, so no working template changes meaning.

3. THE ENGINE RESOLVED PLACEHOLDERS ON READS BUT NOT ON WRITES. 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. 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`.

Note the directions: (3) was fail-closed (matched zero rows); (1) and (2) were
fail-open (matched all). The latter is the data-destroying one.

Author: os-zhuang

.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