From 1ca8666a1bd60602ba4e679eb01a6718d4616db1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 16:30:46 +0000 Subject: [PATCH 1/4] wip(trigger-record-change): decouple flow record from batch payload + adopt #15356 harness Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .../before-update-flow-payload-reach.test.ts | 713 ++++++++++++++++++ .../src/decouple-flow-record.ts | 142 ++++ .../src/record-change-trigger.ts | 55 +- 3 files changed, 904 insertions(+), 6 deletions(-) create mode 100644 packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts create mode 100644 packages/triggers/trigger-record-change/src/decouple-flow-record.ts diff --git a/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts b/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts new file mode 100644 index 0000000000..0b6a6c2885 --- /dev/null +++ b/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts @@ -0,0 +1,713 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15356] MEASUREMENT — can a `record-before-update` flow reach the BATCH + * PAYLOAD of a `multi: true` update? + * + * ## Why this file exists + * + * #14744's census found the in-repo population of same-key / per-row-VALUE + * `beforeUpdate` payload rewrites is ZERO across 23 production registration + * sites. One door that zero does not bound was named there but never driven: + * `record-change-trigger.ts`'s `start()` binds `beforeUpdate` for the + * `record-before-update` / `record-before-write` trigger types and hands the + * write to USER-AUTHORED FLOW METADATA. The conclusion recorded on that card — + * `buildContext` materialises a NEW record object by overlay rather than + * handing the flow `ctx.input.data` by reference, so a flow cannot reach the + * batch payload — was labelled by its own author a SOURCE READING, explicitly + * not a measurement. + * + * This file is the measurement, and it FALSIFIES the reading's conclusion for + * one shape. The overlay is real, and it is a SHALLOW spread + * (`{ ...previous, ...inputData }`): the top-level object is new, and every + * NESTED value in it is the payload's own object, shared by reference. A flow + * write shape that mutates a nested value IN PLACE therefore writes the batch + * payload without ever assigning a top-level key. See `S5` below, measured. + * + * ## The mechanism the probes are aimed at + * + * On a predicate (`multi: true`) update, `ObjectQL.update()`'s predicate branch + * calls `dispatchPerRowBeforeHooks`, whose ADR-0058 Addendum II clause D3 + * hands every per-row context THE batch payload — `rowCtx.input.data` is the + * same object for every row, never a copy — and writes it back onto + * `batchCtx.input.data` after each dispatch. So a `beforeUpdate` handler that + * writes the payload rewrites the SET clause for the WHOLE batch: with rows + * whose pre-images differ, the LAST dispatch's value lands on EVERY row. + * + * ## What the observable is + * + * The batch payload IS the SET clause of the single `driver.updateMany`, so + * "reached the payload" is directly readable off the persisted rows: a value + * that appears on EVERY row although only one row's dispatch produced it can + * have arrived only through the shared payload. Each stack ALSO carries a + * witness `beforeUpdate` hook at priority 1000 (lower runs first, so it runs + * after the record-change trigger's own handler) that snapshots + * `ctx.input.data` per row — a direct read of the payload after the flow has + * run, independent of what the driver then does with it. + * + * ## Every probe proves its own flow RAN + * + * `RecordChangeTrigger`'s handler swallows flow failures by design (error + * isolation — a flow must never break the CRUD write). A probe that measured + * only "no residue" could therefore be reporting a flow that never fired. So + * every probe asserts a POSITIVE trace of its own run — an audit row the flow + * wrote, or the script function's own recorded observation — before it is + * allowed to report "does not reach". + * + * ## Two controls, because a negative needs them + * + * - `positive control` — a script `beforeUpdate` hook that DOES assign the + * payload (the #14744 pinned residue shape), in this same harness, showing + * the last dispatch's value on every row. Without it firing, every "does not + * reach" below would be a claim about this harness, not about flows. + * - `#14099 armed control` — a hook writing DIVERGENT KEY SETS per row must + * be refused whole, so "the refusal did not fire for the nested shape" is a + * measurement rather than an unarmed check. + * + * ⚠️ #15356 is a MEASUREMENT card: no guard, no write-shape change, no ADR. + * The fix side is on the maintainer floor (ADR-0058 Addendum II D3). + */ +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 type { IDataEngine, IObjectQLEngine } from '@objectstack/spec/contracts'; +// `check:test-source-alias` — this package resolves `@objectstack/driver-sql` +// through `dist/`, so its first load must be paid during COLLECTION, not inside +// a clocked test body: a `it(…, 25000)` that also transforms a dependency's +// whole module graph measures loading, not behaviour. +import { SqlDriver } from '@objectstack/driver-sql'; +import { RecordChangeTriggerPlugin } from './plugin.js'; + +/** + * `registerObject` / `registerHook` / `registerFunction` are concrete-engine + * seams the `objectql` slot's published contract does not model (the same + * narrowing `bulk-write-per-row-context.test.ts` in this package makes for + * `registerObject`), declared structurally so the slot lookup itself stays + * fully typed (#4127/#4251) instead of reaching for `any`. + */ +type TestObjectRegistry = { + registerObject(schema: unknown, packageId?: string, namespace?: string): void; +}; +type TestHookSurface = { + registerHook( + event: string, + handler: (ctx: any) => unknown | Promise, + options?: { object?: string | string[]; priority?: number; packageId?: string }, + ): void; + registerFunction(name: string, handler: (...args: any[]) => unknown, packageId?: string): void; +}; + +const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +/** Memory driver with a real `updateMany` (ONE SET clause for N rows). */ +function makeDriver(): any { + const stores = new Map>>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + 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; + }; + const sel = (o: string, ast: any) => [...storeFor(o).values()].filter((r) => matches(r, ast?.where)); + 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 }; storeFor(o).set(id, full); return { ...full }; + }, + async update(o: string, id: string, data: any) { + const cur = storeFor(o).get(id) ?? {}; const u = { ...cur, ...data, id }; + storeFor(o).set(id, u); return { ...u }; + }, + // The caller's bound is honoured, applied AFTER the filter and by PRESENCE + // (`check:objectql-double-limit`): a double that silently ignores `limit` + // answers a question the engine did not ask. + async find(o: string, ast: any, opts?: { limit?: number }) { + // The bound is read by PRESENCE (`limit: 0` must return nothing, not + // everything) from EITHER position it can arrive in — the engine passes + // driver options third, the AST-shaped call carries it second. + const bound = typeof opts?.limit === 'number' + ? opts.limit + : typeof ast?.limit === 'number' ? ast.limit : undefined; + const rows = sel(o, ast); + const page = typeof bound === 'number' ? rows.slice(0, bound) : rows; + return page.map((r) => ({ ...r })); + }, + async findOne(o: string, ast: any) { const [r] = sel(o, ast); return r ? { ...r } : null; }, + async delete(o: string, id: string) { return storeFor(o).delete(id); }, + async count(o: string, ast: any) { return sel(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 updateMany(o: string, ast: any, data: any) { + const rows = sel(o, ast); + // The SET clause is applied to every matched row — deep-copied on the way + // in so the stored rows do not alias the payload and the assertions below + // read what was WRITTEN, not what was mutated afterwards. + const set = JSON.parse(JSON.stringify(data ?? {})); + for (const r of rows) storeFor(o).set(r.id as string, { ...r, ...set, id: r.id }); + return rows.length; + }, + async deleteMany(o: string, ast: any) { + const rows = sel(o, ast); + for (const r of rows) storeFor(o).delete(r.id as string); + return rows.length; + }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; +} + +/** + * `residue` and `tags` are DECLARED fields on purpose: the engine's + * declared-field door (#8738 pre-hook / #13657 post-hook) refuses a payload + * carrying an undeclared key, so a probe writing an undeclared name would be + * measuring that refusal instead of the reach question. + * + * `tags` is a `multiselect` — an ARRAY-valued declared field, which is what + * gives the payload a NESTED value at all. `buildContext`'s overlay is a + * SHALLOW spread, so a flow write shape able to mutate a nested value in place + * reaches the payload through reference sharing without assigning any + * top-level key. That is the falsification candidate, and it cannot be tried + * on a payload of scalars. + */ +function registerObjects(registry: TestObjectRegistry, object: string): void { + registry.registerObject({ + name: object, label: object, + fields: { + title: { name: 'title', label: 'Title', type: 'text' }, + status: { name: 'status', label: 'Status', type: 'text' }, + residue: { name: 'residue', label: 'Residue', type: 'text' }, + tags: { name: 'tags', label: 'Tags', type: 'multiselect' }, + }, + }, 'test', 'test'); + registry.registerObject({ + name: `${object}_audit`, label: 'audit', + fields: { + seen: { name: 'seen', label: 'Seen', type: 'text' }, + note: { name: 'note', label: 'Note', type: 'text' }, + }, + }, 'test', 'test'); +} + +interface WitnessEntry { + id: unknown; + data: Record; + payloadRef: unknown; + tagsRef: unknown; +} + +interface Stack { + data: IDataEngine; + automation: AutomationEngine; + objectql: IObjectQLEngine & TestHookSurface; + /** Per-row snapshots of `ctx.input.data`, taken AFTER the trigger's handler. */ + witness: WitnessEntry[]; +} + +async function bootStack(object: string): Promise { + const kernel = new ObjectKernel({ logger: { level: '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 IObjectQLEngine & TestHookSurface; + const data = kernel.getService('data'); + const automation = kernel.getService('automation'); + objectql.registerDriver(makeDriver(), true); + registerObjects(objectql.registry as unknown as TestObjectRegistry, object); + + // Priority 1000 (`entries.sort((a, b) => a.priority - b.priority)` — lower + // runs FIRST), so the witness observes the payload after the record-change + // trigger's handler, and therefore after the flow that handler awaited. + const witness: WitnessEntry[] = []; + objectql.registerHook( + 'beforeUpdate', + (ctx: any) => { + const payload = ctx.input?.data; + witness.push({ + id: ctx.input?.id, + data: JSON.parse(JSON.stringify(payload ?? {})), + payloadRef: payload, + tagsRef: payload?.tags, + }); + }, + { object, priority: 1000, packageId: 'test:witness' }, + ); + + return { data, automation, objectql, witness }; +} + +/** Two rows whose pre-images DIFFER — one batch cannot separate the residue from the clock. */ +async function seedTwoRows(data: IDataEngine, object: string): Promise { + await data.insert(object, [ + { title: 'alpha', status: 'todo' }, + { title: 'beta', status: 'blocked' }, + ], { context: { userId: 'u1' } }); +} + +/** The ONE predicate write every probe measures. */ +async function batchUpdate( + data: IDataEngine, + object: string, + payload: Record, +): Promise<{ ok: boolean; error?: string }> { + try { + await data.update(object, payload, { multi: true, where: {}, context: { userId: 'u1' } }); + return { ok: true }; + } catch (err) { + return { ok: false, error: (err as Error)?.message ?? String(err) }; + } +} + +const rowsByTitle = async (data: IDataEngine, object: string) => { + const rows: any[] = await data.find(object, {}); + return new Map(rows.map((r) => [r.title, r])); +}; + +/** + * A probe flow: [start on record-before-update] → [the shape under test] → + * [log] → [end]. The `log` node is the flow's own proof of life, and it sits + * AFTER the probe node, so a probe node that failed its own config takes the + * audit row with it rather than reporting a silent "does not reach". + */ +function probeFlow(name: string, object: string, probeNode: Record) { + return { + name, label: name, type: 'record_change', + nodes: [ + { + id: 'start', type: 'start', label: 'Start', + config: { objectName: object, triggerType: 'record-before-update' }, + }, + probeNode, + { + id: 'log', type: 'create_record', label: 'Log', + config: { + objectName: `${object}_audit`, + fields: { seen: '{previous.title}', note: '{previous.status}' }, + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: probeNode.id as string }, + { id: 'e2', source: probeNode.id as string, target: 'log' }, + { id: 'e3', source: 'log', target: 'end' }, + ], + }; +} + +describe('[#15356] can a record-before-update flow reach a multi:true batch payload?', () => { + it('⭐ POSITIVE CONTROL: a script beforeUpdate hook assigning the payload lands the LAST dispatch\'s value on EVERY row', async () => { + const object = 'pc'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + + // The residue shape: a per-row-VALUE write of the SAME key on every row. + // The key SET is identical across rows, so #14099's divergence refusal does + // not fire — that is precisely the blind spot #14744 is weighing. + const dispatched: string[] = []; + stack.objectql.registerHook( + 'beforeUpdate', + (ctx: any) => { + const seen = String(ctx.previous?.title ?? '?'); + dispatched.push(seen); + ctx.input.data.residue = `from:${seen}`; + }, + { object, priority: 10, packageId: 'test:residue' }, + ); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote, `the control's own write must succeed: ${wrote.error}`).toMatchObject({ ok: true }); + expect(dispatched.sort(), 'one dispatch per row, on differing pre-images').toEqual(['alpha', 'beta']); + + const rows = await rowsByTitle(stack.data, object); + const residues = [rows.get('alpha')?.residue, rows.get('beta')?.residue]; + // ⭐ The defect, observed: ONE value — the last dispatch's — on BOTH rows. + expect(new Set(residues).size, `residues=${JSON.stringify(residues)}`).toBe(1); + expect(residues[0]).toBe(`from:${dispatched[dispatched.length - 1]}`); + // Every per-row context carried the SAME payload object (D3). + expect(new Set(stack.witness.map((w) => w.payloadRef)).size).toBe(1); + expect(stack.witness[stack.witness.length - 1]?.data.residue).toBe(residues[0]); + }, 20000); + + it('⭐ #14099 ARMED CONTROL: divergent key sets across rows refuse the batch whole', async () => { + const object = 'kd'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + stack.objectql.registerHook( + 'beforeUpdate', + (ctx: any) => { + // Row 'alpha' writes `residue`; row 'beta' writes nothing — divergent + // key SETS, which is what #14099 refuses. + if (ctx.previous?.title === 'alpha') ctx.input.data.residue = 'x'; + }, + { object, priority: 10, packageId: 'test:divergent' }, + ); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote.ok, 'the refusal must be ARMED in this harness').toBe(false); + expect(wrote.error).toMatch(/residue/); + // Refused BEFORE any write: neither row moved. + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.status).toBe('todo'); + expect(rows.get('beta')?.status).toBe('blocked'); + }, 20000); + + it('A2.4 — the write-shape vocabulary is READ OFF THE LIVE REGISTRY, not remembered', async () => { + const stack = await bootStack('vocab'); + const types = ((stack.automation as any).getRegisteredNodeTypes() as string[]).sort(); + // ADR-0018 makes this registry — not a closed enum — the authority on what + // a node `type` may be, so the shape enumeration is derived from it. + // eslint-disable-next-line no-console + console.log('[#15356] registered node types:', JSON.stringify(types)); + // The types with any write sink at all. `start`/`end` are structural + // (handled with no executor); every other registered type's ONLY variable + // sink is `variables.set(, value)` — a whole-name write into the run's + // Map, which cannot address a key inside `record` or the payload. + expect(types).toContain('assignment'); + expect(types).toContain('script'); + expect(types).toContain('update_record'); + }, 20000); + + it('S1 assignment node (canonical `assignments` map) — does NOT reach the payload', async () => { + const object = 's1'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s1_flow', probeFlow('s1_flow', object, { + id: 'probe', type: 'assignment', label: 'Assign', + config: { assignments: { residue: 'REACHED-{previous.title}' } }, + }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(200); + + const audit: any[] = await stack.data.find(`${object}_audit`, {}); + expect(audit.map((r) => r.seen).sort(), 'the flow must have RUN, once per row').toEqual(['alpha', 'beta']); + + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.residue ?? null).toBeNull(); + expect(rows.get('beta')?.residue ?? null).toBeNull(); + expect(stack.witness.every((w) => !('residue' in w.data))).toBe(true); + }, 20000); + + it('S2 assignment node with a DOTTED target (`record.residue`) — does NOT reach the payload', async () => { + const object = 's2'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s2_flow', probeFlow('s2_flow', object, { + id: 'probe', type: 'assignment', label: 'Assign', + // A dotted name is a variable literally NAMED `record.residue` — the run's + // variables are a flat `Map`, so this addresses no path inside `record`. + config: { assignments: { 'record.residue': 'REACHED', 'input.data.residue': 'REACHED' } }, + }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(200); + + const audit: any[] = await stack.data.find(`${object}_audit`, {}); + expect(audit.map((r) => r.seen).sort()).toEqual(['alpha', 'beta']); + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.residue ?? null).toBeNull(); + expect(rows.get('beta')?.residue ?? null).toBeNull(); + expect(stack.witness.every((w) => !('residue' in w.data))).toBe(true); + }, 20000); + + it('S3 assignment node REPLACING the `record` variable — does NOT reach the payload', async () => { + const object = 's3'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s3_flow', probeFlow('s3_flow', object, { + id: 'probe', type: 'assignment', label: 'Assign', + config: { assignments: { record: { residue: 'REACHED', status: 'HIJACKED' } } }, + }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(200); + + const audit: any[] = await stack.data.find(`${object}_audit`, {}); + expect(audit.map((r) => r.seen).sort(), 'the flow must have RUN, once per row').toEqual(['alpha', 'beta']); + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.residue ?? null).toBeNull(); + expect(rows.get('alpha')?.status).toBe('done'); + expect(stack.witness.every((w) => !('residue' in w.data))).toBe(true); + }, 20000); + + it('S4 script node — a function assigning `automation.record.` does NOT reach the payload', async () => { + const object = 's4'; + const stack = await bootStack(object); + const seen: Array> = []; + stack.objectql.registerFunction('mutate_top_level', async (args: any) => { + const rec = args.automation?.record as Record; + seen.push({ + // What handles does a script function actually get? Recorded, not assumed. + automationKeys: Object.keys(args.automation ?? {}).sort(), + argKeys: Object.keys(args ?? {}).sort(), + sameAsVariable: args.variables?.get('record') === rec, + previousTitle: (args.automation?.previous as any)?.title, + // #4862 fact 4, re-checked rather than inherited: `title` is NOT in + // this write's payload, so a `record` that is the BARE payload could + // not supply it. A per-row value here says `record` is the row's own + // state, folded from its pre-image. + recordTitle: rec?.title, + }); + rec.residue = 'REACHED'; + return { ok: true }; + }); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s4_flow', probeFlow('s4_flow', object, { + id: 'probe', type: 'script', label: 'Script', + config: { function: 'mutate_top_level', inputs: {} }, + }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(200); + + // eslint-disable-next-line no-console + console.log('[#15356] S4 script function saw:', JSON.stringify(seen)); + expect(seen, 'the script function must have RUN, once per row').toHaveLength(2); + // #4862 re-check on today's tree: `previous` IS bound, and it is THIS row's + // pre-image — the two rows started in different states and each run saw its own. + expect(seen.map((s) => s.previousTitle).sort()).toEqual(['alpha', 'beta']); + // #4862 fact 4, re-checked: `record` is NOT the bare payload — it carries + // this row's own `title`, a field this write never mentioned. + expect(seen.map((s) => s.recordTitle).sort()).toEqual(['alpha', 'beta']); + // The `record` CEL root and the flow's `record` variable are ONE object. + expect(seen.every((s) => s.sameAsVariable === true)).toBe(true); + // The function is handed no payload handle at all: `AutomationContext` + // declares no `input`/`data` member, and the arg bag carries only these four. + expect(seen[0]?.argKeys).toEqual(['automation', 'input', 'logger', 'variables']); + expect(seen[0]?.automationKeys as string[]).not.toContain('input'); + + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.residue ?? null).toBeNull(); + expect(rows.get('beta')?.residue ?? null).toBeNull(); + expect(stack.witness.every((w) => !('residue' in w.data))).toBe(true); + }, 20000); + + /** + * ⭐ S5 — THE FALSIFICATION. ⚠️ This test asserts a DEFECT as measured on + * 2026-09-04, not a property anyone wants: a red here means the door was + * CLOSED (by a copy at `buildContext`'s overlay, a freeze, or a guard), and + * the repair is to convert this into a cannot-reach pin and update #14744 / + * #15356 — ⛔ never to weaken the assertion. + */ + it('S5 ⭐ script node mutating a NESTED payload value IN PLACE — REACHES the batch payload', async () => { + const object = 's5'; + const stack = await bootStack(object); + const observed: Array> = []; + const flowSideRefs: unknown[] = []; + stack.objectql.registerFunction('mutate_nested', async (args: any) => { + const rec = args.automation?.record as Record; + const tags = rec?.tags; + flowSideRefs.push(tags); + observed.push({ + tagsIsArray: Array.isArray(tags), + tagsAsSeen: JSON.stringify(tags ?? null), + previousTitle: (args.automation?.previous as any)?.title, + }); + // An in-place mutation of a NESTED value. No top-level key is assigned, + // so the #14088 payload-write recorder — which observes `set` traps on + // the payload object — records nothing. + if (Array.isArray(tags)) tags.push(`REACHED-${(args.automation?.previous as any)?.title}`); + return { ok: true }; + }); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s5_flow', probeFlow('s5_flow', object, { + id: 'probe', type: 'script', label: 'Script', + config: { function: 'mutate_nested', inputs: {} }, + }) as any); + + // The payload carries a NESTED (array) value — without one there is no + // reference to share and this probe would measure nothing. + const wrote = await batchUpdate(stack.data, object, { status: 'done', tags: ['seed'] }); + await sleep(200); + + expect(observed, 'the script function must have RUN, once per row').toHaveLength(2); + // The flow saw row 1's mutation already present when row 2 dispatched — + // the two runs share ONE array, which is the batch payload's own. + expect(observed[0]?.tagsAsSeen).toBe('["seed"]'); + expect(observed[1]?.tagsAsSeen).toBe('["seed","REACHED-alpha"]'); + // Reference identity, measured across the boundary: the array the flow + // mutated IS the array on `ctx.input.data`. + expect(stack.witness.length).toBe(2); + expect(flowSideRefs[0]).toBe(stack.witness[0]?.tagsRef); + expect(flowSideRefs[1]).toBe(stack.witness[1]?.tagsRef); + expect(new Set(stack.witness.map((w) => w.payloadRef)).size).toBe(1); + + // ⭐ The reach, on the persisted rows: BOTH rows carry BOTH dispatches' + // contributions, including the one derived from the OTHER row's pre-image. + expect(wrote, 'and #14099 did NOT refuse it').toMatchObject({ ok: true }); + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.tags).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); + expect(rows.get('beta')?.tags).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); + }, 20000); + + it('S5b the same nested mutation on a BY-ID update stays on its own row — the harm is multi-specific', async () => { + const object = 's5b'; + const stack = await bootStack(object); + stack.objectql.registerFunction('mutate_nested_single', async (args: any) => { + const tags = (args.automation?.record as any)?.tags; + if (Array.isArray(tags)) tags.push('REACHED'); + return { ok: true }; + }); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s5b_flow', probeFlow('s5b_flow', object, { + id: 'probe', type: 'script', label: 'Script', + config: { function: 'mutate_nested_single', inputs: {} }, + }) as any); + + const before = await rowsByTitle(stack.data, object); + await stack.data.update( + object, + { id: before.get('alpha')?.id, status: 'done', tags: ['seed'] } as any, + { context: { userId: 'u1' } }, + ); + await sleep(200); + + const rows = await rowsByTitle(stack.data, object); + // The mutation still reaches THIS write's payload — the aliasing is not + // multi-specific — but a by-id payload names one row, so nothing leaks. + expect(rows.get('alpha')?.tags).toEqual(['seed', 'REACHED']); + expect(rows.get('beta')?.tags ?? null).toBeNull(); + expect(rows.get('beta')?.status).toBe('blocked'); + }, 20000); + + it('S6 update_record node aimed at the triggering row — a SEPARATE write, not the batch payload', async () => { + const object = 's6'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s6_flow', probeFlow('s6_flow', object, { + id: 'probe', type: 'update_record', label: 'Update', + config: { + objectName: object, + filter: { id: '{record.id}' }, + fields: { residue: 'REACHED-{previous.title}' }, + }, + }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(400); + + const rows = await rowsByTitle(stack.data, object); + // PER-ROW values, so this did NOT travel through the shared batch payload — + // it is the node's own by-id write, aimed at the row the flow ran for. + expect(rows.get('alpha')?.residue).toBe('REACHED-alpha'); + expect(rows.get('beta')?.residue).toBe('REACHED-beta'); + // And the batch payload the witness saw never carried `residue`. + const batchWitness = stack.witness.filter((w) => 'status' in w.data); + expect(batchWitness.length).toBeGreaterThan(0); + expect(batchWitness.every((w) => !('residue' in w.data))).toBe(true); + }, 20000); + + it('S7 update_record forwarding `{record.tags}` into ANOTHER write — does NOT mutate the batch payload', async () => { + const object = 's7'; + const stack = await bootStack(object); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('s7_flow', probeFlow('s7_flow', object, { + id: 'probe', type: 'update_record', label: 'Update', + config: { + objectName: object, + filter: { id: '{record.id}' }, + // The object token resolves to the array itself; the question is + // whether the second write's own passes mutate the shared array. + fields: { tags: '{record.tags}', residue: 'copied' }, + }, + }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done', tags: ['seed'] }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(400); + + const batchWitness = stack.witness.filter((w) => 'status' in w.data); + expect(batchWitness.length).toBeGreaterThan(0); + // The batch payload's array is untouched by the forwarded copy. + expect(batchWitness.every((w) => JSON.stringify(w.data.tags) === '["seed"]')).toBe(true); + const rows = await rowsByTitle(stack.data, object); + expect(rows.get('alpha')?.tags).toEqual(['seed']); + expect(rows.get('beta')?.tags).toEqual(['seed']); + }, 20000); +}); + +/** + * [#15356] S5 again, on the REAL SQL backend — `@objectstack/driver-sql` over + * better-sqlite3 `:memory:`, built the canonical way this package's + * `record-change-integration.test.ts` boots it. + * + * The reach itself is already proven above by reference identity measured + * across the boundary (the array the flow mutated IS the array on + * `ctx.input.data`), which no driver can affect. What only a real driver can + * settle is the CONSEQUENCE the card names: one SET clause, issued once, so + * the accumulated mutation lands on EVERY matched row of a real `UPDATE`. + */ +describe('[#15356] S5 on the real SQL driver — the reach lands on every row of a real UPDATE', () => { + it('a flow script mutating `record.tags` in place writes both dispatches onto both rows', async () => { + const kernel = new ObjectKernel({ logger: { level: '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 IObjectQLEngine & TestHookSurface & { + syncSchemas(): Promise; + }; + const data = kernel.getService('data'); + const automation = kernel.getService('automation'); + + const driver: any = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.connect(); + objectql.registerDriver(driver, true); + registerObjects(objectql.registry as unknown as TestObjectRegistry, 'sq'); + await objectql.syncSchemas(); + + objectql.registerFunction('sq_mutate_nested', async (args: any) => { + const tags = (args.automation?.record as any)?.tags; + if (Array.isArray(tags)) tags.push(`REACHED-${(args.automation?.previous as any)?.title}`); + return { ok: true }; + }); + automation.registerFlow('sq_flow', probeFlow('sq_flow', 'sq', { + id: 'probe', type: 'script', label: 'Script', + config: { function: 'sq_mutate_nested', inputs: {} }, + }) as any); + + await seedTwoRows(data, 'sq'); + const wrote = await batchUpdate(data, 'sq', { status: 'done', tags: ['seed'] }); + await sleep(300); + + const audit: any[] = await data.find('sq_audit', {}); + expect(audit.map((r) => r.seen).sort(), 'the flow must have RUN, once per row').toEqual(['alpha', 'beta']); + expect(wrote, 'and #14099 did NOT refuse it').toMatchObject({ ok: true }); + + const rows = await rowsByTitle(data, 'sq'); + // eslint-disable-next-line no-console + console.log('[#15356] SQL rows:', JSON.stringify([...rows.values()].map((r) => ({ title: r.title, tags: r.tags })))); + const alpha = rows.get('alpha')?.tags; + const beta = rows.get('beta')?.tags; + expect(alpha).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); + expect(beta).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); + + await driver.disconnect?.(); + }, 25000); +}); diff --git a/packages/triggers/trigger-record-change/src/decouple-flow-record.ts b/packages/triggers/trigger-record-change/src/decouple-flow-record.ts new file mode 100644 index 0000000000..c27bea2538 --- /dev/null +++ b/packages/triggers/trigger-record-change/src/decouple-flow-record.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Decouple the flow-facing `record` / `previous` roots from the ENGINE-OWNED + * objects they were overlaid from (#14744, measured by #15356). + * + * ## The leak this closes + * + * `buildContext` builds the flow's `record` as an overlay — + * `{ ...previous, ...inputData, ...after }`. The top-level object is new, so a + * flow that ASSIGNS a top-level key writes only that new object. But the spread + * is SHALLOW: every NESTED value in it is the engine's own object, shared by + * reference. `inputData` is `ctx.input.data`, and on a predicate (`multi: true`) + * write ADR-0058 Addendum II D3 hands EVERY per-row context THE SAME payload + * object — it is the SET clause of the single `driver.updateMany`. + * + * So a `script` node whose registered function mutated a nested value in place + * (`automation.record.tags.push(…)`) wrote the batch payload without assigning + * any key, and every dispatch's contribution landed on EVERY matched row — + * including a value derived from another row's pre-image. #14099's key-set + * refusal cannot see it: that refusal reads the set of keys each row's chain + * ASSIGNED (via #14088's `set`-trap recorder), and an in-place mutation of a + * nested value fires no trap, so both rows report the EMPTY key set and there is + * nothing to diverge. Measured end to end on the memory driver and on + * `@objectstack/driver-sql` (#15356; the pin is + * `before-update-flow-payload-reach.test.ts`, S5 in this package). + * + * `ctx.previous` is the same shape of leak one seam over: the engine binds ONE + * pre-image object and hands the SAME `HookContext` to every OTHER flow binding + * on the write, which is why `buildContext` already refuses to materialise into + * it. That copy was shallow too, so the refusal only held for top-level keys. + * + * ## Why a COPY and not a FREEZE + * + * The ruling names both. A freeze is not available here, and the reason is + * measured rather than aesthetic: `service-automation`'s `expandDeclaredLookups` + * (#3475) writes `record[field] = expanded` INTO the context this function + * returns — its own docblock says "Mutates `record` in place (the same object + * the run's variable map already references)" — and the expander is wired in + * every deployment that has objectql. Under a freeze that assignment throws, the + * best-effort `catch` swallows it, and every flow declaring `config.expand` + * silently degrades to unexpanded scalar ids while logging "could not expand + * lookups". A freeze also converts one unsupported write into a whole-flow + * outage, because `RecordChangeTrigger`'s handler swallows flow failures by + * design (error isolation — a flow must never break the CRUD write). + * + * A copy keeps the flow's own view live: the mutation still takes effect on the + * snapshot the flow is holding, so `{record.tags}` later in the SAME run still + * observes it. What it can no longer do is reach the engine's write. + * + * ## The boundary, stated because it is not total + * + * Copied: arrays, plain objects (prototype `Object.prototype` or `null`), + * `Date`, `RegExp`, `Map`, `Set` — the mutable shapes a record value can + * actually arrive as. Shared by reference: primitives (nothing to alias), + * functions, and any other class instance — copying an exotic instance by + * property assignment corrupts it worse than sharing it does (internal slots, + * private fields), and `buildContext` must never break the flow it feeds. A + * declared field's value is JSON-shaped by its field type, so that residue is + * not a shape any driver produces for a record; it is named here rather than + * left as an unknown, and `decouple-flow-record.test.ts` pins it in both + * directions. + * + * Cross-realm values (a `Date` from another `vm` context) fail `instanceof` and + * fall into the shared-by-reference arm — that is today's behaviour, so the + * fallback can only ever be a non-improvement, never a regression. + * + * The `seen` map does two jobs: it terminates on cycles, and — when the SAME + * map is passed for `record` and `previous` — it keeps substructure the two + * roots shared with each other shared WITHIN the flow's own context, so the + * copy changes what the flow can reach OUTWARD without rearranging what it sees + * INWARD. It also means a repeated reference is walked once, not once per site. + */ + +/** + * Return a copy of `value` that shares no mutable object with the engine's own + * state. Not exported from `index.ts` — module scope only, like + * {@link materializeDeclaredFields}, so this stays off the package's published + * API surface. + */ +export function decoupleFromEngineState(value: T, seen: WeakMap = new WeakMap()): T { + return copyValue(value, seen) as T; +} + +function copyValue(value: unknown, seen: WeakMap): unknown { + // Primitives and functions: nothing to alias, or nothing safely copyable. + if (value === null || typeof value !== 'object') return value; + + // Every value this function registers is a non-`undefined` object, so a + // `get` miss and a stored copy are distinguishable without a second lookup. + const already = seen.get(value); + if (already !== undefined) return already; + + if (Array.isArray(value)) { + const out: unknown[] = new Array(value.length); + // Registered BEFORE the walk so a cycle resolves to this same array. + seen.set(value, out); + for (let i = 0; i < value.length; i += 1) out[i] = copyValue(value[i], seen); + return out; + } + + if (value instanceof Date) { + const out = new Date(value.getTime()); + seen.set(value, out); + return out; + } + + if (value instanceof RegExp) { + const out = new RegExp(value.source, value.flags); + out.lastIndex = value.lastIndex; + seen.set(value, out); + return out; + } + + if (value instanceof Map) { + const out = new Map(); + seen.set(value, out); + for (const [k, v] of value) out.set(copyValue(k, seen), copyValue(v, seen)); + return out; + } + + if (value instanceof Set) { + const out = new Set(); + seen.set(value, out); + for (const v of value) out.add(copyValue(v, seen)); + return out; + } + + const proto = Object.getPrototypeOf(value) as unknown; + if (proto === Object.prototype || proto === null) { + const out: Record = {}; + seen.set(value, out); + for (const key of Object.keys(value)) { + out[key] = copyValue((value as Record)[key], seen); + } + return out; + } + + // The documented residue: a class instance is shared, not copied. + seen.set(value, value); + return value; +} 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 7bd70565b4..001287ce8e 100644 --- a/packages/triggers/trigger-record-change/src/record-change-trigger.ts +++ b/packages/triggers/trigger-record-change/src/record-change-trigger.ts @@ -3,6 +3,8 @@ import type { AutomationContext } from '@objectstack/spec/contracts'; import type { HookContext } from '@objectstack/spec/data'; +import { decoupleFromEngineState } from './decouple-flow-record.js'; + /** * Structural mirror of the automation engine's `FlowTriggerBinding` * (service-automation/src/engine.ts). Declared locally so this trigger plugin @@ -117,7 +119,9 @@ export interface TriggerLogger { * Mutates `record` in place (matching the canonical copy's contract) and * returns it. Callers that must not mutate a shared object — this file's own * `ctx.previous`, observed by every OTHER binding sharing the same - * HookContext — pass a shallow copy in. + * HookContext — pass a copy in. ⚠️ A SHALLOW copy is enough for THIS function + * (it only ever assigns top-level keys), and it is NOT enough for the object + * that reaches a flow: see {@link decoupleFromEngineState} and #14744. * * Exported (module-scope only — NOT re-exported from `index.ts`, so this * stays off the package's published API) so @@ -331,6 +335,19 @@ export class RecordChangeTrigger implements FlowTrigger { * fields (see {@link hydrateComputedFields}) via a data-engine re-read, * AND — since #4953 (services half) — made total over the object's * declared fields (see the `materializeDeclaredFields` call below). + * + * ⭐ Both roots it returns are a SNAPSHOT and are DECOUPLED from the + * engine's own state (#14744): a flow can mutate them however it likes and + * reach nothing outside its own run. Until #14744 that was true only of the + * TOP LEVEL — every overlay here is a shallow spread, so each nested value + * was still the engine's own object, and `inputData` is the batch payload + * ADR-0058 Addendum II D3 shares across every row of a `multi: true` write. + * The reading "buildContext materialises a NEW record object by overlay, so + * a flow cannot reach `ctx.input.data`" was therefore true of the object and + * FALSE of its contents; #15356 measured a `script` node's registered + * function reaching the payload through a nested in-place mutation, and the + * `decoupleFromEngineState` call below is what makes the sentence true as + * stated. A flow that needs to WRITE uses the `update_record` node. */ private async buildContext(binding: FlowTriggerBinding, ctx: HookContext): Promise { // objectql lifecycle hooks carry the written row under `input.data` (insert / @@ -427,13 +444,36 @@ export class RecordChangeTrigger implements FlowTrigger { // is handed to every OTHER flow binding on this write (see the class doc // on `hydrationCache`), so writing into it would leak materialised // `null`s into bindings that haven't run yet. Same rule - // `readonlyWhenBindings` follows for the identical reason. + // `readonlyWhenBindings` follows for the identical reason. This spread + // guards the KEY SET only; the `decoupleFromEngineState` call below is + // what makes the no-write-through rule hold for nested values too. const materializedPrevious = priorBase && fields ? materializeDeclaredFields({ ...priorBase }, fields) : previous; + // #14744 — DECOUPLE the flow-facing roots from the engine's own objects. + // Every overlay above is a SHALLOW spread, so until this point each + // nested value in `record` is still the engine's: `inputData` is + // `ctx.input.data`, which ADR-0058 Addendum II D3 shares across every + // per-row context of a `multi: true` write AS the SET clause of the one + // `driver.updateMany`. A flow function mutating a nested value in place + // therefore wrote the batch payload while assigning no key at all — + // invisible to #14099's key-set refusal, and landing every dispatch's + // contribution on every matched row (measured, #15356 S5; pinned in + // `before-update-flow-payload-reach.test.ts`). ⛔ Not a widening of + // #14099: that refusal is untouched, and this closes the ALIASING the + // refusal was never instrumented to see. A flow that needs to write the + // record uses the `update_record` node, which issues its own by-id write + // (S6). ONE `seen` map for both roots, so substructure the two shared + // with EACH OTHER stays shared inside the flow's own context — the copy + // changes what a flow can reach outward, not what it observes inward. + // See `decouple-flow-record.ts` for why a copy and not a freeze. + const isolation = new WeakMap(); + const isolatedRecord = decoupleFromEngineState(hydrated, isolation); + const isolatedPrevious = decoupleFromEngineState(materializedPrevious, isolation); + return { - record: hydrated, - previous: materializedPrevious, + record: isolatedRecord, + previous: isolatedPrevious, object, event: binding.event, userId: session.userId, @@ -452,8 +492,11 @@ export class RecordChangeTrigger implements FlowTrigger { // driver-layer `tenantId` field unchanged. ...(session.organizationId ? { tenantId: session.organizationId } : {}), // Expose the record as params too, so flows with named `isInput` - // variables matching record fields get them seeded. - params: hydrated, + // variables matching record fields get them seeded. Deliberately the + // SAME object as `record` (unchanged by #14744 — `params` was never a + // second snapshot, and making it one here would be an observable + // change on top of the aliasing fix). + params: isolatedRecord, }; } From c3ce250bf75e762ae51777f5c403a95967204a5d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 16:44:30 +0000 Subject: [PATCH 2/4] test(trigger-record-change): flip S5/S5b and the SQL replica to cannot-reach pins Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .../before-update-flow-payload-reach.test.ts | 146 ++++++++++++------ 1 file changed, 102 insertions(+), 44 deletions(-) diff --git a/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts b/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts index 0b6a6c2885..7737da84db 100644 --- a/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts +++ b/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts @@ -1,8 +1,28 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#15356] MEASUREMENT — can a `record-before-update` flow reach the BATCH - * PAYLOAD of a `multi: true` update? + * [#15356 measured, #14744 closed] PIN — a `record-before-update` flow reaches + * NO write shape into the BATCH PAYLOAD of a `multi: true` update. + * + * ## What changed about this file, and what did not + * + * It was written for #15356 as a MEASUREMENT and it answered NOT BOUNDED: one + * shape (`S5`) reached the payload. #14744 then ruled the door closed — + * `buildContext` decouples the flow-facing roots from the engine's own objects + * (`decoupleFromEngineState`) — and this file was adopted whole as the pin. + * `S5`, `S5b` and the SQL replica were FLIPPED to their opposites in that same + * PR, keeping every observable they measured on: reference identity across the + * boundary, the per-row readings, and the persisted rows. ⛔ Nothing was + * weakened, and nothing was deleted: the paragraphs below still describe the + * defect as it was measured, because a pin whose reader cannot see what it is + * pinning against is a pin nobody dares repair. + * + * The two controls are why the negatives are readable, and BOTH must keep + * firing: the positive control (a script hook that ASSIGNS the payload — the + * #14744 residue shape — still lands the LAST dispatch's value on every row, + * because #14744's fix is about aliasing and deliberately does not touch that + * residue) and the #14099 armed control (divergent key sets are still refused + * whole). If either stops firing, this file has stopped measuring. * * ## Why this file exists * @@ -17,12 +37,19 @@ * batch payload — was labelled by its own author a SOURCE READING, explicitly * not a measurement. * - * This file is the measurement, and it FALSIFIES the reading's conclusion for - * one shape. The overlay is real, and it is a SHALLOW spread - * (`{ ...previous, ...inputData }`): the top-level object is new, and every - * NESTED value in it is the payload's own object, shared by reference. A flow - * write shape that mutates a nested value IN PLACE therefore writes the batch - * payload without ever assigning a top-level key. See `S5` below, measured. + * This file is the measurement, and it FALSIFIED the reading's conclusion for + * one shape. The overlay was real, and it was a SHALLOW spread + * (`{ ...previous, ...inputData }`): the top-level object was new, and every + * NESTED value in it was the payload's own object, shared by reference. A flow + * write shape that mutated a nested value IN PLACE therefore wrote the batch + * payload without ever assigning a top-level key. See `S5` below. + * + * ⭐ #14744 made the reading's sentence TRUE AS STATED rather than deleting it: + * the overlay still materialises a new object, and `decoupleFromEngineState` + * now makes that true of the object's CONTENTS too. The distinction is worth + * keeping in front of the next reader — "a new object" and "reaches nothing" + * were two different claims for the whole life of this package, and only one of + * them was ever checked. * * ## The mechanism the probes are aimed at * @@ -64,8 +91,12 @@ * be refused whole, so "the refusal did not fire for the nested shape" is a * measurement rather than an unarmed check. * - * ⚠️ #15356 is a MEASUREMENT card: no guard, no write-shape change, no ADR. - * The fix side is on the maintainer floor (ADR-0058 Addendum II D3). + * ⚠️ #15356 was a MEASUREMENT card: no guard, no write-shape change, no ADR. + * #14744 carries the fix, and it is still not a write-shape change: ADR-0058 + * Addendum II D3 stands untouched — the engine does not split its own write, + * one payload still serves N rows, and every per-row context is still handed + * that one object (asserted in `S5`). What changed is only that the object a + * FLOW is handed no longer shares anything with it. */ import { describe, it, expect } from 'vitest'; import { ObjectKernel } from '@objectstack/core'; @@ -503,13 +534,19 @@ describe('[#15356] can a record-before-update flow reach a multi:true batch payl }, 20000); /** - * ⭐ S5 — THE FALSIFICATION. ⚠️ This test asserts a DEFECT as measured on - * 2026-09-04, not a property anyone wants: a red here means the door was - * CLOSED (by a copy at `buildContext`'s overlay, a freeze, or a guard), and - * the repair is to convert this into a cannot-reach pin and update #14744 / - * #15356 — ⛔ never to weaken the assertion. + * ⭐ S5 — THE PIN. This case was written on 2026-09-04 as a CHARACTERISATION + * of the defect (the nested in-place mutation REACHED the payload, and both + * rows carried both dispatches' contributions). #14744 closed the door on the + * same day by decoupling the flow-facing roots from the engine's own objects + * (`decoupleFromEngineState`, called at the end of `buildContext`), and the + * case was flipped in the same PR — the assertions below are the OPPOSITE of + * what they were, not a weakened version of them. + * + * ⚠️ A red here now means the door was RE-OPENED: some overlay on the way to + * the flow started sharing a nested reference with `ctx.input.data` again. + * The repair is at that overlay, ⛔ never in this file. */ - it('S5 ⭐ script node mutating a NESTED payload value IN PLACE — REACHES the batch payload', async () => { + it('S5 ⭐ script node mutating a NESTED payload value IN PLACE — CANNOT reach the batch payload', async () => { const object = 's5'; const stack = await bootStack(object); const observed: Array> = []; @@ -541,26 +578,35 @@ describe('[#15356] can a record-before-update flow reach a multi:true batch payl await sleep(200); expect(observed, 'the script function must have RUN, once per row').toHaveLength(2); - // The flow saw row 1's mutation already present when row 2 dispatched — - // the two runs share ONE array, which is the batch payload's own. + // Row 2 does NOT see row 1's mutation: each dispatch is handed its own copy + // of the nested value, so neither run can observe the other's write. Before + // #14744 the second reading was `["seed","REACHED-alpha"]`. expect(observed[0]?.tagsAsSeen).toBe('["seed"]'); - expect(observed[1]?.tagsAsSeen).toBe('["seed","REACHED-alpha"]'); - // Reference identity, measured across the boundary: the array the flow - // mutated IS the array on `ctx.input.data`. + expect(observed[1]?.tagsAsSeen).toBe('["seed"]'); + // Reference identity, measured across the same boundary the defect was + // measured on: the array the flow mutated is NOT the array on + // `ctx.input.data`, and the two rows did not even share it with each other. expect(stack.witness.length).toBe(2); - expect(flowSideRefs[0]).toBe(stack.witness[0]?.tagsRef); - expect(flowSideRefs[1]).toBe(stack.witness[1]?.tagsRef); + expect(flowSideRefs[0]).not.toBe(stack.witness[0]?.tagsRef); + expect(flowSideRefs[1]).not.toBe(stack.witness[1]?.tagsRef); + expect(flowSideRefs[0]).not.toBe(flowSideRefs[1]); + // ⭐ D3 is UNCHANGED and still the reason this matters: every per-row + // context is handed the ONE batch payload object. The fix is the copy on the + // way to the flow, not a split of the engine's write. expect(new Set(stack.witness.map((w) => w.payloadRef)).size).toBe(1); + // Read directly off the payload, after both flows have run: untouched. + expect(stack.witness.every((w) => JSON.stringify(w.data.tags) === '["seed"]')).toBe(true); - // ⭐ The reach, on the persisted rows: BOTH rows carry BOTH dispatches' - // contributions, including the one derived from the OTHER row's pre-image. - expect(wrote, 'and #14099 did NOT refuse it').toMatchObject({ ok: true }); + // ⭐ The persisted rows: the SET clause carries what the CALLER wrote, and + // no row carries a value derived from the other row's pre-image. Before + // #14744 both rows read `['seed','REACHED-alpha','REACHED-beta']`. + expect(wrote, 'and the write still succeeds — this is not a refusal').toMatchObject({ ok: true }); const rows = await rowsByTitle(stack.data, object); - expect(rows.get('alpha')?.tags).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); - expect(rows.get('beta')?.tags).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); + expect(rows.get('alpha')?.tags).toEqual(['seed']); + expect(rows.get('beta')?.tags).toEqual(['seed']); }, 20000); - it('S5b the same nested mutation on a BY-ID update stays on its own row — the harm is multi-specific', async () => { + it('S5b the same nested mutation on a BY-ID update reaches nothing either — the fix is not multi-specific', async () => { const object = 's5b'; const stack = await bootStack(object); stack.objectql.registerFunction('mutate_nested_single', async (args: any) => { @@ -583,9 +629,15 @@ describe('[#15356] can a record-before-update flow reach a multi:true batch payl await sleep(200); const rows = await rowsByTitle(stack.data, object); - // The mutation still reaches THIS write's payload — the aliasing is not - // multi-specific — but a by-id payload names one row, so nothing leaks. - expect(rows.get('alpha')?.tags).toEqual(['seed', 'REACHED']); + // ⚠️ THE BREAKING HALF, pinned deliberately. The aliasing was never + // multi-specific: on a by-id write the same in-place mutation reached this + // write's own payload and PERSISTED correctly (`['seed','REACHED']` before + // #14744), so it read as a working per-row write path rather than as + // corruption. It is the same alias, so closing the door closes it here too, + // and a stack author using it loses a write that used to land. That is why + // the changeset carries a BREAKING banner: the alternative is `update_record` + // (S6), which is a real write and lands per row. + expect(rows.get('alpha')?.tags).toEqual(['seed']); expect(rows.get('beta')?.tags ?? null).toBeNull(); expect(rows.get('beta')?.status).toBe('blocked'); }, 20000); @@ -648,18 +700,21 @@ describe('[#15356] can a record-before-update flow reach a multi:true batch payl }); /** - * [#15356] S5 again, on the REAL SQL backend — `@objectstack/driver-sql` over - * better-sqlite3 `:memory:`, built the canonical way this package's - * `record-change-integration.test.ts` boots it. + * [#15356 measured it, #14744 closed it] S5 again, on the REAL SQL backend — + * `@objectstack/driver-sql` over better-sqlite3 `:memory:`, built the canonical + * way this package's `record-change-integration.test.ts` boots it. * - * The reach itself is already proven above by reference identity measured - * across the boundary (the array the flow mutated IS the array on - * `ctx.input.data`), which no driver can affect. What only a real driver can - * settle is the CONSEQUENCE the card names: one SET clause, issued once, so - * the accumulated mutation lands on EVERY matched row of a real `UPDATE`. + * The reach was proven above by reference identity measured across the + * boundary, which no driver can affect; what only a real driver could settle + * was the CONSEQUENCE the card names — one SET clause, issued once, so the + * accumulated mutation landed on EVERY matched row of a real `UPDATE`. That is + * why the same probe is repeated here rather than trusted from the memory + * double, and it is why it stays here after the fix: a copy that held on the + * memory driver and not on a real `UPDATE` would be the same defect with a + * narrower audience. */ -describe('[#15356] S5 on the real SQL driver — the reach lands on every row of a real UPDATE', () => { - it('a flow script mutating `record.tags` in place writes both dispatches onto both rows', async () => { +describe('[#15356/#14744] S5 on the real SQL driver — the mutation reaches no row of a real UPDATE', () => { + it('a flow script mutating `record.tags` in place leaves both rows carrying the caller\'s value', async () => { const kernel = new ObjectKernel({ logger: { level: 'silent' } }); await kernel.use(new ObjectQLPlugin()); await kernel.use(new AutomationServicePlugin()); @@ -705,8 +760,11 @@ describe('[#15356] S5 on the real SQL driver — the reach lands on every row of console.log('[#15356] SQL rows:', JSON.stringify([...rows.values()].map((r) => ({ title: r.title, tags: r.tags })))); const alpha = rows.get('alpha')?.tags; const beta = rows.get('beta')?.tags; - expect(alpha).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); - expect(beta).toEqual(['seed', 'REACHED-alpha', 'REACHED-beta']); + // Before #14744 both read `['seed','REACHED-alpha','REACHED-beta']` — one + // SET clause carrying both dispatches, including the value derived from the + // other row's pre-image. + expect(alpha).toEqual(['seed']); + expect(beta).toEqual(['seed']); await driver.disconnect?.(); }, 25000); From fd768e867e6ca063506fe7a8c422ca82303914d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 16:48:05 +0000 Subject: [PATCH 3/4] test(trigger-record-change): seam pin for decoupling + expand-graft control Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- .../before-update-flow-payload-reach.test.ts | 62 ++++- .../src/decouple-flow-record.test.ts | 219 ++++++++++++++++++ 2 files changed, 279 insertions(+), 2 deletions(-) create mode 100644 packages/triggers/trigger-record-change/src/decouple-flow-record.test.ts diff --git a/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts b/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts index 7737da84db..4ff6ae259e 100644 --- a/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts +++ b/packages/triggers/trigger-record-change/src/before-update-flow-payload-reach.test.ts @@ -315,13 +315,18 @@ const rowsByTitle = async (data: IDataEngine, object: string) => { * AFTER the probe node, so a probe node that failed its own config takes the * audit row with it rather than reporting a silent "does not reach". */ -function probeFlow(name: string, object: string, probeNode: Record) { +function probeFlow( + name: string, + object: string, + probeNode: Record, + startExtra: Record = {}, +) { return { name, label: name, type: 'record_change', nodes: [ { id: 'start', type: 'start', label: 'Start', - config: { objectName: object, triggerType: 'record-before-update' }, + config: { objectName: object, triggerType: 'record-before-update', ...startExtra }, }, probeNode, { @@ -697,6 +702,59 @@ describe('[#15356] can a record-before-update flow reach a multi:true batch payl expect(rows.get('alpha')?.tags).toEqual(['seed']); expect(rows.get('beta')?.tags).toEqual(['seed']); }, 20000); + + /** + * ⭐ [#14744] THE CONTROL ON THE FIX'S SHAPE — why a COPY and not a FREEZE. + * + * The ruling named deep-copy and freeze as alternatives. They are not + * equivalent, and this case is the measurement that chose between them: + * `service-automation`'s `expandDeclaredLookups` (#3475) writes + * `record[field] = expanded` INTO the context `buildContext` returns — its own + * docblock says "Mutates `record` in place (the same object the run's variable + * map already references)" — and `AutomationServicePlugin` bridges that + * expander in every deployment that has objectql. + * + * Under a deep FREEZE that assignment throws, `expandDeclaredLookups`' + * best-effort `catch` swallows it, and every flow declaring `config.expand` + * silently degrades to unexpanded scalar ids while logging "could not expand + * lookups". Measured: with the freeze variant in `buildContext` this case goes + * RED and the copy keeps it green. That is a shipped feature, so freeze was + * not available and the copy is not a preference. + * + * ⚠️ A red here means the flow-facing `record` stopped accepting the ONE + * in-place write the platform itself performs on it. + */ + it('#14744 — a flow declaring `config.expand` still gets its lookup grafted onto the copy', async () => { + const object = 'exp'; + const stack = await bootStack(object); + const seen: unknown[] = []; + // Stand in for the plugin-bridged expander (whose own wiring and identity + // scoping are pinned by service-automation's `record-lookup-expand` + // integration test); what is under test here is whether its in-place graft + // lands on the object the flow is handed. + stack.automation.setRecordExpander(async () => ({ owner: { id: 'u9', name: 'Owner Nine' } })); + stack.objectql.registerFunction('read_expanded', async (args: any) => { + seen.push((args.automation?.record as Record)?.owner); + return { ok: true }; + }); + await seedTwoRows(stack.data, object); + stack.automation.registerFlow('exp_flow', probeFlow('exp_flow', object, { + id: 'probe', type: 'script', label: 'Script', + config: { function: 'read_expanded', inputs: {} }, + }, { expand: ['owner'] }) as any); + + const wrote = await batchUpdate(stack.data, object, { status: 'done' }); + expect(wrote).toMatchObject({ ok: true }); + await sleep(200); + + expect(seen, 'the script function must have RUN, once per row').toHaveLength(2); + expect(seen[0], 'the expansion must have landed on the record the flow holds').toEqual({ + id: 'u9', name: 'Owner Nine', + }); + expect(seen[1]).toEqual({ id: 'u9', name: 'Owner Nine' }); + // ...and grafting it still reached nothing: `owner` is not in the payload. + expect(stack.witness.every((w) => !('owner' in w.data))).toBe(true); + }, 20000); }); /** diff --git a/packages/triggers/trigger-record-change/src/decouple-flow-record.test.ts b/packages/triggers/trigger-record-change/src/decouple-flow-record.test.ts new file mode 100644 index 0000000000..f3e7cc1095 --- /dev/null +++ b/packages/triggers/trigger-record-change/src/decouple-flow-record.test.ts @@ -0,0 +1,219 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14744] The flow-facing `record` / `previous` roots share no mutable object + * with the engine's own state. + * + * `before-update-flow-payload-reach.test.ts` is the END-TO-END pin: it boots a + * real kernel and measures the consequence on persisted rows. This file is the + * SEAM pin under it, and the two are not redundant — the end-to-end file can + * only exercise the shapes a flow's own vocabulary can produce, while the + * decoupling has to hold for whatever a record VALUE happens to be. + * + * It covers three things that file cannot: + * + * 1. `previous` as well as `record`. `ctx.previous` is the engine's ONE + * pre-image object and the SAME `HookContext` reaches every other flow + * binding on the write, so writing through it leaks sideways rather than + * into the payload. That leak has no persisted-row symptom to measure. + * 2. The value shapes: nested objects, arrays, `Date`, `Map`, `Set`, cycles — + * and the DOCUMENTED RESIDUE, a class instance, which is shared by + * reference on purpose. A boundary nobody asserts is a boundary nobody + * knows has moved. + * 3. That the decoupling did not rearrange what a flow sees INWARD: the two + * roots still share the substructure they shared with each other, and + * `params` is still the same object as `record`. + */ +import { describe, it, expect } from 'vitest'; +import type { AutomationContext } from '@objectstack/spec/contracts'; +import type { HookContext } from '@objectstack/spec/data'; +import { decoupleFromEngineState } from './decouple-flow-record.js'; +import { RecordChangeTrigger, type RecordChangeDataEngine, type TriggerLogger } from './record-change-trigger.js'; + +const silentLogger = (): TriggerLogger => ({ info: () => {}, warn: () => {}, debug: () => {} }); + +/** Fake ObjectQL engine that just captures the registered hook handler. */ +function fakeEngine() { + const hooks: Array<(ctx: HookContext) => unknown | Promise> = []; + const engine: RecordChangeDataEngine = { + registerHook(_event, handler) { + hooks.push(handler); + }, + }; + return { engine, hooks }; +} + +/** + * Drive one `beforeUpdate` dispatch through the real trigger and hand back both + * the context the flow was given and the engine-side objects it was built from, + * so every assertion below is about the SAME pair the engine holds. + */ +async function dispatch( + input: { id?: unknown; data: Record }, + previous: Record, +): Promise<{ flow: AutomationContext; payload: Record; preImage: Record }> { + const { engine, hooks } = fakeEngine(); + const trigger = new RecordChangeTrigger(engine, silentLogger()); + let flow: AutomationContext | undefined; + trigger.start( + { flowName: 'probe', object: 'thing', event: 'record-before-update' }, + async (ctx) => { + flow = ctx; + }, + ); + expect(hooks, 'the trigger must have bound a beforeUpdate hook').toHaveLength(1); + const hookCtx = { + object: 'thing', + event: 'beforeUpdate', + input, + previous, + session: { userId: 'u1' }, + ql: {}, + } as unknown as HookContext; + await hooks[0](hookCtx); + if (!flow) throw new Error('the flow callback never ran — the probe measured nothing'); + return { flow, payload: input.data, preImage: previous }; +} + +describe('[#14744] the record handed to a flow is decoupled from the batch payload', () => { + it('an in-place mutation of a nested payload value does not reach `ctx.input.data`', async () => { + const { flow, payload } = await dispatch( + { id: 't1', data: { status: 'done', tags: ['seed'], meta: { hits: 1 } } }, + { id: 't1', status: 'todo', title: 'alpha' }, + ); + + const record = flow.record as Record; + // The flow sees the payload's values... + expect(record.tags).toEqual(['seed']); + expect(record.meta).toEqual({ hits: 1 }); + // ...through objects that are NOT the engine's. + expect(record.tags).not.toBe(payload.tags); + expect(record.meta).not.toBe(payload.meta); + + (record.tags as string[]).push('REACHED'); + (record.meta as { hits: number }).hits = 99; + + expect(payload.tags, 'the batch payload is the SET clause — it must be untouched').toEqual(['seed']); + expect(payload.meta).toEqual({ hits: 1 }); + // The flow's own view still reflects its own write — a copy, not a + // silent no-op inside the run. + expect(record.tags).toEqual(['seed', 'REACHED']); + }); + + it('a mutation through `previous` does not reach the engine\'s shared pre-image', async () => { + const { flow, preImage } = await dispatch( + { id: 't1', data: { status: 'done' } }, + { id: 't1', status: 'todo', labels: ['old'], nested: { n: 1 } }, + ); + + const previous = flow.previous as Record; + expect(previous.labels).toEqual(['old']); + expect(previous.labels).not.toBe(preImage.labels); + expect(previous.nested).not.toBe(preImage.nested); + + (previous.labels as string[]).push('REACHED'); + (previous.nested as { n: number }).n = 99; + + // `ctx.previous` is handed to every OTHER binding on this write. + expect(preImage.labels).toEqual(['old']); + expect(preImage.nested).toEqual({ n: 1 }); + }); + + it('keeps `params` the same object as `record`, and keeps what the two roots shared with each other', async () => { + const shared = { by: 'u1' }; + const { flow } = await dispatch( + { id: 't1', data: { status: 'done' } }, + { id: 't1', status: 'todo', audit: shared, also: shared }, + ); + + expect(flow.params, '`params` was never a second snapshot').toBe(flow.record); + const previous = flow.previous as Record; + expect(previous.audit, 'one copy, reached twice — not two copies').toBe(previous.also); + expect(previous.audit).not.toBe(shared); + }); +}); + +describe('[#14744] decoupleFromEngineState — the value shapes it covers, and the one it does not', () => { + it('copies arrays, plain objects, Date, RegExp, Map and Set', () => { + const source = { + arr: [1, { deep: 'x' }], + obj: { a: { b: 1 } }, + when: new Date('2026-09-04T00:00:00.000Z'), + re: /abc/gi, + map: new Map([['k', { v: 1 }]]), + set: new Set([{ s: 1 }]), + }; + const copy = decoupleFromEngineState(source); + + expect(copy).toEqual(source); + expect(copy.arr).not.toBe(source.arr); + expect(copy.arr[1]).not.toBe(source.arr[1]); + expect(copy.obj.a).not.toBe(source.obj.a); + expect(copy.when).not.toBe(source.when); + expect(copy.when.getTime()).toBe(source.when.getTime()); + expect(copy.re).not.toBe(source.re); + expect(copy.re.source).toBe('abc'); + expect(copy.re.flags).toBe('gi'); + expect(copy.map).not.toBe(source.map); + expect(copy.map.get('k')).not.toBe(source.map.get('k')); + expect(copy.set).not.toBe(source.set); + expect([...copy.set][0]).not.toBe([...source.set][0]); + + // Mutating every copied container leaves the source alone. + (copy.arr[1] as { deep: string }).deep = 'MUTATED'; + copy.when.setUTCFullYear(1999); + copy.map.set('k2', 1); + copy.set.add('extra'); + expect((source.arr[1] as { deep: string }).deep).toBe('x'); + expect(source.when.toISOString()).toBe('2026-09-04T00:00:00.000Z'); + expect(source.map.size).toBe(1); + expect(source.set.size).toBe(1); + }); + + it('passes primitives and functions through, and terminates on a cycle', () => { + const fn = () => 'kept'; + const cyclic: Record = { n: 1, s: 'x', nil: null, un: undefined, fn }; + cyclic.self = cyclic; + cyclic.list = [cyclic]; + + const copy = decoupleFromEngineState(cyclic); + + expect(copy.n).toBe(1); + expect(copy.s).toBe('x'); + expect(copy.nil).toBeNull(); + expect('un' in copy).toBe(true); + expect(copy.fn, 'a function is shared — there is nothing safe to copy').toBe(fn); + expect(copy.self, 'the cycle resolves to the COPY, not the source').toBe(copy); + expect((copy.list as unknown[])[0]).toBe(copy); + expect(copy).not.toBe(cyclic); + }); + + it('SHARES a class instance by reference — the documented residue, asserted so it cannot move silently', () => { + class Exotic { + constructor(public state: number) {} + bump(): void { + this.state += 1; + } + } + const instance = new Exotic(1); + const copy = decoupleFromEngineState({ instance, wrapped: [instance] }); + + expect(copy.instance, 'shared, deliberately: copying by property assignment breaks internal state').toBe( + instance, + ); + expect((copy.wrapped as Exotic[])[0]).toBe(instance); + // Still a real instance, which is the whole reason it is not copied. + copy.instance.bump(); + expect(instance.state).toBe(2); + }); + + it('walks a repeated reference once when one `seen` map spans both roots', () => { + const shared = { hit: 0 }; + const seen = new WeakMap(); + const a = decoupleFromEngineState({ shared }, seen); + const b = decoupleFromEngineState({ shared }, seen); + + expect(a.shared).not.toBe(shared); + expect(b.shared).toBe(a.shared); + }); +}); From 55145cbad90b27d5a20180f2c61b9ce9cd9ed188 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 17:05:45 +0000 Subject: [PATCH 4/4] chore(changeset): decouple flow record from batch payload Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ --- ...low-record-decoupled-from-batch-payload.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 .changeset/flow-record-decoupled-from-batch-payload.md diff --git a/.changeset/flow-record-decoupled-from-batch-payload.md b/.changeset/flow-record-decoupled-from-batch-payload.md new file mode 100644 index 0000000000..c22cd1cfe5 --- /dev/null +++ b/.changeset/flow-record-decoupled-from-batch-payload.md @@ -0,0 +1,55 @@ +--- +"@objectstack/trigger-record-change": patch +--- + +fix(trigger-record-change)!: the record handed to a record-change flow no longer aliases the write's payload (#14744) + + + +**BREAKING** for a flow whose `script` node mutates a NESTED value of the +triggering record IN PLACE: that mutation no longer affects the write the flow +was triggered by. Shipped as `patch` — this change moves no public surface (no +exported symbol, no accepted key or value), and under the maintainer's +2026-09-04 rule (decision batch #35, on #15294) a `fix(` that changes no public +surface stays `patch`, with breaking-ness carried by this banner and the +ADR-0087 disposition rather than by the level. Maintainer ruling 2026-09-04 on +#14744 (decision batch #38, verbatim 「同意」), adopting option A. + +**Why.** `buildContext` builds the flow's `record` as a shallow overlay of the +pre-image, the mutation payload and the after-row. The top-level object was +new, so a flow ASSIGNING a top-level key reached nothing — but every nested +value in it was the engine's own object, shared by reference. One of those is +`ctx.input.data`, and on a `multi: true` update ADR-0058 Addendum II D3 hands +every per-row context that same payload object, which is the SET clause of the +single `updateMany`. A registered function doing `record.tags.push(...)` +therefore wrote the SET clause without assigning any key: every dispatch's +contribution landed on EVERY matched row, including values derived from another +row's pre-image, and #14099's key-set refusal could not see it because no key +was assigned. Measured end to end on the memory driver and on +`@objectstack/driver-sql` (#15356). + +**What changes.** Both flow-facing roots — `record` (and the `params` alias of +it) and `previous` — are decoupled from the engine's state before the flow +runs. Arrays, plain objects, `Date`, `RegExp`, `Map` and `Set` are copied; +primitives, functions and other class instances are shared, which is the +documented and pinned boundary. A flow still mutates its roots freely and still +observes its own writes for the rest of the run; those writes simply reach +nothing outside it. `previous` is decoupled in the same stroke because it is the +engine's single pre-image object and the same hook context reaches every other +flow bound to the same write. + +**What does NOT change.** The engine's write shape. ADR-0058 Addendum II D3 +stands untouched: one payload still serves N rows and every per-row context is +still handed that one object. #14099's key-set refusal is untouched and is not +widened — a hook that assigns the same key with per-row values still passes it, +and divergent key sets are still refused whole. Flow metadata with no registered +function reached nothing before this change and reaches nothing after it: +assignment nodes write the run's variable map, and `update_record` issues its own +by-id write. Lookup expansion (`config.expand`) still grafts onto the record the +flow holds. + +**Consumer note.** A flow that relied on an in-place nested mutation to persist +— which on a by-id write did persist, and on a `multi: true` write corrupted +every other matched row — writes the record with the `update_record` node +instead. That node is the supported per-row write and is unaffected by this +change.