diff --git a/.changeset/record-change-hydrate-formula-fields.md b/.changeset/record-change-hydrate-formula-fields.md new file mode 100644 index 000000000..0bc253ccf --- /dev/null +++ b/.changeset/record-change-hydrate-formula-fields.md @@ -0,0 +1,36 @@ +--- +"@objectstack/trigger-record-change": patch +--- + +fix(trigger-record-change): hydrate read-time formula fields onto the seeded flow record (#3426) + +A `formula` field is a read-time virtual — the engine evaluates it post-fetch on +`find`/`findOne`, never on the write path — so it was absent from the raw +after-create/after-update row a record-change flow is seeded with. A notify +node template like `{record.full_name}` (or a start condition on the same field) +therefore resolved to an empty string, silently emitting notifications such as +`"New lead to assign: "` with the name missing. + +The record-change trigger now re-reads the just-written record through the data +engine, so the seeded `record` carries the same computed fields a data-API read +returns. The fix is at the trigger (the producer of the flow's `record`), so it +benefits the whole flow — start condition, every node, and notify `title`/`body` +templates — not just the notify node. + +Deliberately conservative: + +- Runs only for `afterInsert` / `afterUpdate` (the row exists in its post-write + state); `before*` and `afterDelete` keep the raw hook record untouched. +- Reads as an elevated system principal, so it can only ADD computed fields, + never let RLS/FLS on the re-read shrink the snapshot the flow already saw. +- Raw hook fields win on merge, preserving trigger-time scalar values and the + #1872 multi-lookup input overlay; the re-read only fills in keys the raw row + lacks (the formula virtuals). +- Any failure (no read surface, no id, a throw, an empty read) falls back to the + raw record — hydration never breaks the flow it feeds. + +Lookup **traversal** (`{record.account.name}`) is intentionally not hydrated: a +default data-API read does not expand relations either, and expanding would turn +`record.account` from its scalar FK id into an object, breaking templates and +conditions that use the bare id (e.g. #1872's `{record.target_channels.0}`). +That traversal remains tracked on #3426. diff --git a/packages/triggers/trigger-record-change/src/formula-context.test.ts b/packages/triggers/trigger-record-change/src/formula-context.test.ts new file mode 100644 index 000000000..870880285 --- /dev/null +++ b/packages/triggers/trigger-record-change/src/formula-context.test.ts @@ -0,0 +1,107 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #3426 — a `formula` field is a READ-time virtual: the engine evaluates it + * post-fetch on `find`/`findOne`, never on the write path, so it is absent from + * the raw after-create/after-update row a record-change flow is seeded with. + * `{record.full_name}` in a notify template (or a start condition) therefore + * resolved to an empty string. The trigger now re-reads the written record + * through the data engine, so the seeded `record` carries the same computed + * fields a data-API read returns. + * + * This exercises the whole stack (real ObjectQL + automation + record-change + * trigger) with a formula field, proving the seeded record resolves it. The + * notify node interpolates the very same variable map, so a formula that + * resolves for `update_record` here resolves for a notify `title`/`body` too. + */ +import { describe, it, expect } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin } from '@objectstack/objectql'; +import { AutomationServicePlugin, type AutomationEngine } from '@objectstack/service-automation'; +import { RecordChangeTriggerPlugin } from './plugin.js'; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +/** Memory driver storing full rows. Formula virtuals are computed by the + * ENGINE post-fetch, never by the driver — so `full_name` is never stored. */ +function makeDriver(): any { + const store = new Map>(); + let n = 0; + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const exp = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (exp ?? null)) return false; + } + return true; + }; + return { + name: 'memory', version: '0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, async syncSchema() {}, + async create(_o: string, data: any) { + n += 1; const id = data.id ?? `r_${n}`; + const full = { ...data, id }; + store.set(id, full); + return { ...full }; + }, + async update(_o: string, id: string, data: any) { const cur = store.get(id) ?? {}; const u = { ...cur, ...data, id }; store.set(id, u); return { ...u }; }, + async find(_o: string, ast: any) { return [...store.values()].filter((r) => matches(r, ast?.where)).map((r) => ({ ...r })); }, + async findOne(_o: string, ast: any) { for (const r of store.values()) if (matches(r, ast?.where)) return { ...r }; return null; }, + async delete(_o: string, id: string) { return store.delete(id); }, + async count(_o: string, ast: any) { return (await this.find(_o, ast)).length; }, + async upsert(_o: string, d: any) { return this.create(_o, d); }, + async bulkCreate(_o: string, rows: any[]) { return Promise.all(rows.map((r) => this.create(_o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; +} + +describe('record-change context hydrates read-time formula fields (#3426)', () => { + it('resolves a formula field ({record.full_name}) in a seeded flow record', async () => { + const kernel = new ObjectKernel({ logLevel: 'silent' }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AutomationServicePlugin()); + await kernel.use(new RecordChangeTriggerPlugin()); + await kernel.bootstrap(); + + const objectql = kernel.getService('objectql') as any; + const data = kernel.getService('data') as any; + const automation = kernel.getService('automation'); + objectql.registerDriver(makeDriver(), true); + objectql.registry.registerObject({ + name: 'crm_lead', label: 'Lead', + fields: { + first_name: { name: 'first_name', label: 'First', type: 'text' }, + last_name: { name: 'last_name', label: 'Last', type: 'text' }, + // Read-time formula virtual — never present on the raw written row. + full_name: { + name: 'full_name', label: 'Full name', type: 'formula', + expression: { dialect: 'cel', source: 'record.first_name + " " + record.last_name' }, + }, + greeting: { name: 'greeting', label: 'Greeting', type: 'text' }, + }, + }, 'test', 'test'); + + // On create, stamp `greeting` from the formula field. Before the fix this + // stamped '' because `{record.full_name}` was blank in the seeded record. + automation.registerFlow('lead_greeting', { + name: 'lead_greeting', label: 'Greeting', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { objectName: 'crm_lead', triggerType: 'record-after-create' } }, + { id: 'stamp', type: 'update_record', label: 'Stamp', config: { objectName: 'crm_lead', filter: { id: '{record.id}' }, fields: { greeting: 'Hello, {record.full_name}!' } } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ { id: 'e1', source: 'start', target: 'stamp' }, { id: 'e2', source: 'stamp', target: 'end' } ], + } as any); + + const created = await data.insert('crm_lead', { first_name: 'Ada', last_name: 'Lovelace' }); + const id = Array.isArray(created) ? created[0]?.id : created?.id ?? created; + await sleep(200); + const row = await data.findOne('crm_lead', { where: { id } }); + expect(row?.full_name).toBe('Ada Lovelace'); // read-path sanity + expect(row?.greeting).toBe('Hello, Ada Lovelace!'); // flow saw the formula + }, 15000); +}); diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts index 54b55fe11..29b434c40 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.test.ts @@ -258,6 +258,148 @@ describe('RecordChangeTrigger', () => { }); }); +// ─── computed-field hydration (#3426) ─────────────────────────────── + +describe('RecordChangeTrigger computed-field hydration (#3426)', () => { + interface FindOneCall { + object: string; + options: { where?: Record; fields?: string[]; context?: unknown }; + } + + /** fakeEngine + a `findOne` that records its calls and returns `row`. */ + function fakeEngineWithRead(row: Record | null | undefined) { + const base = fakeEngine(); + const calls: FindOneCall[] = []; + const engine: RecordChangeDataEngine = { + ...base.engine, + async findOne(object, options) { + calls.push({ object, options }); + return row; + }, + }; + return { engine, hooks: base.hooks, calls }; + } + + it('hydrates a formula field the raw hook row lacks (after-create)', async () => { + // The re-read returns the formula virtual `full_name` (absent from the + // written row) plus a field only the read path carries. After the merge + // the flow sees the formula, and raw scalars still win. + const { engine, hooks, calls } = fakeEngineWithRead({ + id: 'r1', + first_name: 'Ada', + last_name: 'Lovelace', + full_name: 'Ada Lovelace', + company: 'Analytical Engines', + }); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let captured: AutomationContext | undefined; + + trigger.start( + binding({ object: 'crm_lead', event: 'record-after-create' }), + async (ctx) => { captured = ctx; }, + ); + await hooks[0].handler( + hookCtx({ event: 'afterInsert', result: { id: 'r1', first_name: 'Ada', last_name: 'Lovelace' } }), + ); + + // Formula virtual is now resolvable on the seeded record… + expect((captured?.record as Record).full_name).toBe('Ada Lovelace'); + expect((captured?.record as Record).company).toBe('Analytical Engines'); + // …and params mirrors the same hydrated record. + expect((captured?.params as Record)?.full_name).toBe('Ada Lovelace'); + // Re-read was a system-elevated findOne scoped to the written row. + expect(calls).toHaveLength(1); + expect(calls[0].object).toBe('crm_lead'); + expect(calls[0].options.where).toEqual({ id: 'r1' }); + expect((calls[0].options.context as { isSystem?: boolean }).isSystem).toBe(true); + }); + + it('lets raw hook fields win over the re-read (trigger-time fidelity + #1872)', async () => { + // A concurrent read could observe a newer scalar or drop a multi-lookup + // array the driver echoed into the raw row; the raw value must survive. + const { engine, hooks } = fakeEngineWithRead({ + id: 'r1', + status: 'stale', + target_channels: undefined, + full_name: 'Ada Lovelace', + }); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let captured: AutomationContext | undefined; + + trigger.start(binding({ object: 'crm_lead', event: 'record-after-update' }), async (ctx) => { captured = ctx; }); + await hooks[0].handler( + hookCtx({ event: 'afterUpdate', result: { id: 'r1', status: 'fresh', target_channels: ['ch_1'] } }), + ); + + const rec = captured?.record as Record; + expect(rec.status).toBe('fresh'); // raw wins + expect(rec.target_channels).toEqual(['ch_1']); // #1872 array preserved + expect(rec.full_name).toBe('Ada Lovelace'); // formula added + }); + + it('does not re-read for before-* events (row not yet persisted)', async () => { + const { engine, hooks, calls } = fakeEngineWithRead({ id: 'r1', full_name: 'x' }); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + + trigger.start(binding({ object: 'crm_lead', event: 'record-before-update' }), async () => {}); + await hooks[0].handler(hookCtx({ event: 'beforeUpdate', result: { id: 'r1' } })); + + expect(calls).toHaveLength(0); + }); + + it('does not re-read for after-delete (row is gone)', async () => { + const { engine, hooks, calls } = fakeEngineWithRead({ id: 'r1', full_name: 'x' }); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + + trigger.start(binding({ object: 'crm_lead', event: 'record-after-delete' }), async () => {}); + await hooks[0].handler(hookCtx({ event: 'afterDelete', result: { id: 'r1' } })); + + expect(calls).toHaveLength(0); + }); + + it('does not re-read when the record has no id', async () => { + const { engine, hooks, calls } = fakeEngineWithRead({ id: 'r1', full_name: 'x' }); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let captured: AutomationContext | undefined; + + trigger.start(binding({ object: 'crm_lead', event: 'record-after-create' }), async (ctx) => { captured = ctx; }); + await hooks[0].handler(hookCtx({ event: 'afterInsert', result: { first_name: 'Ada' } })); + + expect(calls).toHaveLength(0); + expect((captured?.record as Record).first_name).toBe('Ada'); + }); + + it('falls back to the raw record when the re-read throws', async () => { + const base = fakeEngine(); + const engine: RecordChangeDataEngine = { + ...base.engine, + async findOne() { throw new Error('db down'); }, + }; + const debug = vi.fn(); + const trigger = new RecordChangeTrigger(engine, { info: () => {}, warn: () => {}, debug }); + let captured: AutomationContext | undefined; + + trigger.start(binding({ object: 'crm_lead', event: 'record-after-create' }), async (ctx) => { captured = ctx; }); + await base.hooks[0].handler(hookCtx({ event: 'afterInsert', result: { id: 'r1', first_name: 'Ada' } })); + + // Flow still runs with the raw record; the failure is a debug note only. + expect((captured?.record as Record).first_name).toBe('Ada'); + expect((captured?.record as Record).full_name).toBeUndefined(); + expect(debug).toHaveBeenCalled(); + }); + + it('is a no-op on engines with no findOne surface (older cores)', async () => { + const { engine, hooks } = fakeEngine(); // no findOne + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let captured: AutomationContext | undefined; + + trigger.start(binding({ object: 'crm_lead', event: 'record-after-create' }), async (ctx) => { captured = ctx; }); + await hooks[0].handler(hookCtx({ event: 'afterInsert', result: { id: 'r1', first_name: 'Ada' } })); + + expect((captured?.record as Record).first_name).toBe('Ada'); + }); +}); + // ─── RecordChangeTriggerPlugin ────────────────────────────────────── describe('RecordChangeTriggerPlugin', () => { diff --git a/packages/triggers/trigger-record-change/src/record-change-trigger.ts b/packages/triggers/trigger-record-change/src/record-change-trigger.ts index c0a1e26da..aefbda45b 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -51,6 +51,20 @@ export interface RecordChangeDataEngine { * (2026-07-17 third-party eval). */ getObject?(name: string): unknown; + /** + * Optional record re-read (the ObjectQL engine's `findOne`). When present, + * {@link RecordChangeTrigger} uses it to hydrate the seeded `record` with + * the read-time computed fields the raw lifecycle-hook row never carries — + * chiefly `formula` virtual fields, which are evaluated post-fetch on the + * READ path, not stored on the row. Without this, a flow's start condition + * and every `{record.}` template that names a formula field resolve + * empty (#3426). Signature mirrors `IDataEngine.findOne`; typed structurally + * so this plugin keeps its zero build-time dependency on objectql. + */ + findOne?( + object: string, + options: { where?: Record; fields?: string[]; context?: unknown }, + ): Promise | null | undefined>; } /** Minimal logger surface (matches core's `ctx.logger`). */ @@ -157,7 +171,7 @@ export class RecordChangeTrigger implements FlowTrigger { if ((ctx.session as { skipTriggers?: boolean } | undefined)?.skipTriggers) { return; } - const automationCtx = this.buildContext(binding, ctx); + const automationCtx = await this.buildContext(binding, ctx); await callback(automationCtx); } catch (err) { // Error isolation: a flow failure must NEVER break the CRUD write @@ -199,8 +213,11 @@ export class RecordChangeTrigger implements FlowTrigger { * record comes from `ctx.result` (after-hooks) or falls back to the * mutation input doc / previous row; the old record from `ctx.previous` * (with the `__previous` stash audit also uses as a fallback). + * + * Async because the seeded `record` is hydrated with read-time computed + * fields (see {@link hydrateComputedFields}) via a data-engine re-read. */ - private buildContext(binding: FlowTriggerBinding, ctx: HookContext): AutomationContext { + private async buildContext(binding: FlowTriggerBinding, ctx: HookContext): Promise { // objectql lifecycle hooks carry the written row under `input.data` (insert / // update payload); `id` is on update. (`doc` kept only as a defensive alias.) const input = (ctx.input ?? {}) as { data?: Record; doc?: Record; id?: unknown }; @@ -227,10 +244,17 @@ export class RecordChangeTrigger implements FlowTrigger { const session = (ctx.session ?? {}) as { userId?: string; organizationId?: string }; + const object = binding.object ?? ctx.object; + + // Hydrate read-time computed fields (formula virtuals) onto the seeded + // record so the flow's start condition and every `{record.}` + // template resolve them — the raw hook row never carries them (#3426). + const hydrated = await this.hydrateComputedFields(object, ctx.event, record); + return { - record, + record: hydrated, previous, - object: binding.object ?? ctx.object, + object, event: binding.event, userId: session.userId, // Forward the writer's identity so a `runAs:'user'` flow enforces RLS @@ -249,7 +273,64 @@ export class RecordChangeTrigger implements FlowTrigger { ...(session.organizationId ? { tenantId: session.organizationId } : {}), // Expose the record as params too, so flows with named `isInput` // variables matching record fields get them seeded. - params: record, + params: hydrated, }; } + + /** + * Re-read the just-written record through the data engine so the seeded + * `record` carries the SAME read-time computed fields the data API returns — + * chiefly `formula` virtual fields, which lifecycle-hook rows never include + * because they are evaluated on the READ path, not stored on the row + * (#3426). Without this, `{record.full_name}` (a formula) in a notify + * template, or a start condition referencing one, silently renders blank. + * + * Deliberately conservative: + * - Runs only for `afterInsert` / `afterUpdate`, where the row exists in + * its post-write state. `before*` rows are not yet persisted and + * `afterDelete` rows are gone; both keep the raw hook record untouched. + * - Reads as an elevated SYSTEM principal so it can only ADD computed + * fields, never let RLS/FLS on the re-read shrink the snapshot the flow + * was already going to see from the raw (unmasked) write-path row. + * - Raw hook fields WIN over the re-read on merge, preserving trigger-time + * scalar values and the #1872 multi-lookup input overlay; the re-read + * only fills in keys the raw row lacks (the formula virtuals). + * - Lookup TRAVERSAL (`{record.account.name}`) is intentionally NOT + * hydrated: a default data-API read does not expand relations either, and + * expanding would turn `record.account` from its scalar FK id into an + * object, breaking templates/conditions that use the bare id (e.g. + * #1872's `{record.target_channels.0}`). Tracked separately on #3426. + * + * Any failure (no read surface, no id, a throw, an empty read) falls back to + * the raw record — hydration must never break the flow it feeds. + */ + private async hydrateComputedFields( + object: string | undefined, + hookEvent: string | undefined, + record: Record, + ): Promise> { + if (typeof this.engine.findOne !== 'function') return record; + if (hookEvent !== 'afterInsert' && hookEvent !== 'afterUpdate') return record; + if (!object) return record; + const id = (record as { id?: unknown }).id; + if (id == null || id === '') return record; + try { + const full = await this.engine.findOne(object, { + where: { id }, + // Elevated read: adds computed fields without RLS/FLS masking the + // ones the raw row already carried. + context: { isSystem: true, positions: [], permissions: [] }, + }); + if (full && typeof full === 'object') { + // Raw hook fields win; the re-read only contributes keys the raw + // row lacks (the formula virtuals + any other read-time field). + return { ...(full as Record), ...record }; + } + } catch (err) { + this.logger.debug?.( + `[record-change] computed-field hydration skipped for '${object}': ${(err as Error)?.message ?? String(err)}`, + ); + } + return record; + } }