Skip to content

Commit b0b71a6

Browse files
committed
feat(automation): surface silently-stripped write fields as step warnings (#3407)
update_record reported an unconditional success even when the data layer legally stripped the requested write fields (static readonly #2948, conditional readonlyWhen #3042) — the only trace was a server-side logger warn, invisible in the flow run trace, which is how #3356's approval stage write-backs failed behind a clean 3ms success. - spec: new DroppedFieldsEventSchema ({object, fields, reason}) in data/data-engine.zod.ts (Zod-first, JSON-representable, published to the schema manifest) + a WriteObservabilityOptions mixin (onFieldsDropped listener) on IDataEngine.insert/update option params in contracts/data-engine.ts. The listener is TS-contract-level and in-process only — a function is unrepresentable in JSON Schema and never crosses the RPC boundary. Core's mirror contract kept in sync. - objectql: engine.update() reports each strip pass's dropped keys + reason through options.onFieldsDropped at all four strip sites (single-id + bulk x readonly + readonly_when), diffing before/after key sets (exact: strips return the same reference when nothing dropped). A throwing listener never breaks the write. System-context writes skip the readonly strip and report nothing, as before. insert() accepts the option for signature symmetry but strips nothing today (INSERT is readonly-exempt; FLS write denial throws). - service-automation: NodeExecutionResult and StepLogEntry gain advisory warnings?: string[] (persisted through run history; success semantics unchanged). update_record / create_record attach one warning per strip event naming the dropped fields and expose a structured droppedFields output ({<nodeId>.droppedFields}) for downstream nodes. Design per issue #3407 direction 1 (engine structured feedback) — chosen over run-scoped logger routing (direction 3): shallower intrusion, and the node gets a structured field list instead of log text. FLS is not wired: its write gate now throws (fail-closed) instead of stripping, so it is no longer a silent-drop case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HaofCZbsPTHE2oJedvKd77
1 parent b132181 commit b0b71a6

12 files changed

Lines changed: 604 additions & 39 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/service-automation": minor
5+
---
6+
7+
feat(automation): surface silently-stripped write fields as step warnings (#3407)
8+
9+
`update_record` used to report an unconditional `success` even when the data
10+
layer legally stripped the requested write fields — static `readonly` (#2948)
11+
or a TRUE `readonlyWhen` predicate (#3042). The only trace was a server-side
12+
logger warn, invisible in the flow run trace: an author saw a clean 3ms
13+
`success` while the DB truth never changed (how #3356's approval stage
14+
write-backs failed unnoticed).
15+
16+
- **spec**: new `DroppedFieldsEventSchema` / `DroppedFieldsEvent`
17+
(`{ object, fields, reason: 'readonly' | 'readonly_when' }`) in
18+
`data/data-engine.zod.ts`, and a `WriteObservabilityOptions`
19+
(`onFieldsDropped` listener) mixin on `IDataEngine.insert/update` option
20+
params in `contracts/data-engine.ts`. The listener is a TS-contract-level,
21+
in-process-only channel — deliberately NOT part of the serializable Zod
22+
options schemas or the RPC boundary.
23+
- **objectql**: `engine.update()` reports each strip pass's dropped keys +
24+
reason through `options.onFieldsDropped` (all four strip sites: single-id +
25+
bulk × readonly + readonlyWhen). A throwing listener never breaks the write.
26+
System-context writes skip the readonly strip and therefore report nothing,
27+
as before. `insert()` accepts the option for symmetry but strips nothing
28+
today (INSERT is readonly-exempt; FLS write denial throws).
29+
- **service-automation**: `NodeExecutionResult` and `StepLogEntry` gain
30+
advisory `warnings?: string[]`; `update_record` / `create_record` attach one
31+
warning per strip event naming the dropped fields, plus a structured
32+
`droppedFields` output (`{<nodeId>.droppedFields}`) for downstream nodes.
33+
`success` semantics are unchanged — stripping stays legal, it just is no
34+
longer silent.

content/docs/kernel/contracts/data-engine.mdx

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ export interface IDataEngine {
3737
count(objectName: string, query?: EngineCountOptions): Promise<number>;
3838
aggregate(objectName: string, query: EngineAggregateOptions): Promise<any[]>;
3939

40-
// Mutation
41-
insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any>;
42-
update(objectName: string, data: any, options?: EngineUpdateOptions): Promise<any>;
40+
// Mutation (write ops also accept in-process WriteObservabilityOptions — see `update`)
41+
insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise<any>;
42+
update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise<any>;
4343
delete(objectName: string, options?: EngineDeleteOptions): Promise<any>;
4444

4545
// AI / Vector Search (optional)
@@ -222,6 +222,36 @@ interface EngineUpdateOptions {
222222
}
223223
```
224224

225+
### WriteObservabilityOptions
226+
227+
The write methods (`insert` / `update`) additionally accept an **in-process**
228+
`onFieldsDropped` listener. The engine invokes it when caller-supplied write
229+
fields are legally stripped from the payload before the driver write — static
230+
`readonly` fields or a TRUE `readonlyWhen` predicate. The write still succeeds;
231+
the listener exists so callers that report per-field success (e.g. the flow
232+
engine's `update_record` step) can surface a warning instead of a silent
233+
success.
234+
235+
```typescript
236+
interface WriteObservabilityOptions {
237+
onFieldsDropped?: (event: DroppedFieldsEvent) => void;
238+
}
239+
240+
interface DroppedFieldsEvent {
241+
object: string; // resolved object name
242+
fields: string[]; // caller-supplied fields that were dropped
243+
reason: 'readonly' | 'readonly_when'; // why they were dropped
244+
}
245+
```
246+
247+
<Callout type="warn">
248+
`onFieldsDropped` is a **TS-contract-level, in-process-only** channel. It is
249+
deliberately not part of the serializable Zod options schemas: a function is
250+
unrepresentable in JSON Schema and cannot cross the RPC (Virtual Data Engine)
251+
boundary, so remote callers never receive these events. A listener that throws
252+
never breaks the write — the engine catches and logs.
253+
</Callout>
254+
225255
### delete
226256

227257
Deletes record(s) matching the `where` condition.

packages/core/src/contracts/data-engine.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,50 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
import {
3+
import {
44
EngineQueryOptions,
5-
DataEngineInsertOptions,
6-
EngineUpdateOptions,
5+
DataEngineInsertOptions,
6+
EngineUpdateOptions,
77
EngineDeleteOptions,
8-
EngineAggregateOptions,
8+
EngineAggregateOptions,
99
EngineCountOptions,
1010
DataEngineRequest,
11+
DroppedFieldsEvent,
1112
} from '@objectstack/spec/data';
1213

14+
/**
15+
* In-process write-observability hooks for `insert`/`update` (#3407).
16+
* Mirror of `WriteObservabilityOptions` in `@objectstack/spec/contracts` —
17+
* see that definition for the full rationale (in-process only; never part of
18+
* the serializable options schemas or the RPC boundary).
19+
*/
20+
export interface WriteObservabilityOptions {
21+
/** Called once per strip pass that dropped ≥1 caller-supplied field. */
22+
onFieldsDropped?: (event: DroppedFieldsEvent) => void;
23+
}
24+
1325
/**
1426
* IDataEngine - Standard Data Engine Interface
15-
*
27+
*
1628
* Abstract interface for data persistence capabilities.
1729
* Following the Dependency Inversion Principle - plugins depend on this interface,
1830
* not on concrete database implementations.
19-
*
31+
*
2032
* All query methods use standard QueryAST parameter names
2133
* (where/fields/orderBy/limit/offset/expand) to eliminate mechanical translation
2234
* between the Engine and Driver layers.
23-
*
35+
*
2436
* Aligned with 'src/data/data-engine.zod.ts' in @objectstack/spec.
2537
*/
2638

2739
export interface IDataEngine {
2840
find(objectName: string, query?: EngineQueryOptions): Promise<any[]>;
2941
findOne(objectName: string, query?: EngineQueryOptions): Promise<any>;
30-
insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any>;
31-
update(objectName: string, data: any, options?: EngineUpdateOptions): Promise<any>;
42+
insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise<any>;
43+
update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise<any>;
3244
delete(objectName: string, options?: EngineDeleteOptions): Promise<any>;
3345
count(objectName: string, query?: EngineCountOptions): Promise<number>;
3446
aggregate(objectName: string, query: EngineAggregateOptions): Promise<any[]>;
35-
47+
3648
/**
3749
* Vector Search (AI/RAG)
3850
*/
@@ -48,5 +60,3 @@ export interface IDataEngine {
4860
*/
4961
execute?(command: any, options?: Record<string, any>): Promise<any>;
5062
}
51-
52-

packages/objectql/src/engine.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,112 @@ describe('ObjectQL Engine', () => {
10021002
});
10031003
});
10041004

1005+
/**
1006+
* #3407 — the readonly/readonlyWhen strips are legal semantics, but they
1007+
* must not be SILENT to the caller: `options.onFieldsDropped` reports each
1008+
* strip pass's dropped keys + reason so callers (flow `update_record`
1009+
* steps) can surface a warning on an otherwise-successful write.
1010+
*/
1011+
describe('Dropped-field write observability (#3407)', () => {
1012+
beforeEach(async () => {
1013+
engine.registerDriver(mockDriver, true);
1014+
await engine.init();
1015+
(mockDriver as any).updateMany = vi.fn().mockResolvedValue(1);
1016+
});
1017+
1018+
const docSchema = {
1019+
name: 'doc',
1020+
fields: {
1021+
title: { type: 'text' },
1022+
created_by: { type: 'text', readonly: true },
1023+
},
1024+
} as any;
1025+
1026+
it('reports caller-supplied static-readonly fields stripped on a single-id update', async () => {
1027+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
1028+
const events: any[] = [];
1029+
await engine.update('doc', { id: '1', title: 'x', created_by: 'attacker' }, {
1030+
onFieldsDropped: (e: any) => events.push(e),
1031+
} as any);
1032+
1033+
expect(events).toEqual([{ object: 'doc', fields: ['created_by'], reason: 'readonly' }]);
1034+
// The strip behavior itself is unchanged: the write proceeded without the field.
1035+
const [, , data] = vi.mocked(mockDriver.update).mock.calls[0];
1036+
expect(data).not.toHaveProperty('created_by');
1037+
expect(data).toHaveProperty('title', 'x');
1038+
});
1039+
1040+
it('reports a readonlyWhen-locked field on a single-id update (reason readonly_when)', async () => {
1041+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({
1042+
name: 'invoice',
1043+
fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' } },
1044+
} as any);
1045+
vi.mocked(mockDriver.findOne).mockResolvedValue({ id: '1', locked: true, amount: 100 } as any);
1046+
1047+
const events: any[] = [];
1048+
await engine.update('invoice', { id: '1', amount: 999 }, {
1049+
onFieldsDropped: (e: any) => events.push(e),
1050+
} as any);
1051+
1052+
expect(events).toEqual([{ object: 'invoice', fields: ['amount'], reason: 'readonly_when' }]);
1053+
const [, , data] = vi.mocked(mockDriver.update).mock.calls[0];
1054+
expect(data).not.toHaveProperty('amount');
1055+
});
1056+
1057+
it('reports bulk-path strips too (multi update — locked in ≥1 matched row)', async () => {
1058+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({
1059+
name: 'invoice',
1060+
fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' } },
1061+
} as any);
1062+
vi.mocked(mockDriver.find).mockResolvedValue([
1063+
{ id: 'a', locked: false, amount: 10 },
1064+
{ id: 'b', locked: true, amount: 20 },
1065+
] as any);
1066+
1067+
const events: any[] = [];
1068+
await engine.update('invoice', { amount: 999 }, {
1069+
where: { status: 'draft' }, multi: true,
1070+
onFieldsDropped: (e: any) => events.push(e),
1071+
} as any);
1072+
1073+
expect(events).toEqual([{ object: 'invoice', fields: ['amount'], reason: 'readonly_when' }]);
1074+
const [, , data] = (mockDriver as any).updateMany.mock.calls[0];
1075+
expect(data).not.toHaveProperty('amount');
1076+
});
1077+
1078+
it('does NOT report for a system-context update (the readonly strip is skipped)', async () => {
1079+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
1080+
const events: any[] = [];
1081+
await engine.update('doc', { id: '1', created_by: 'importer' }, {
1082+
context: { isSystem: true },
1083+
onFieldsDropped: (e: any) => events.push(e),
1084+
} as any);
1085+
expect(events).toEqual([]);
1086+
// System writes legitimately set read-only columns — kept, not stripped.
1087+
const [, , data] = vi.mocked(mockDriver.update).mock.calls[0];
1088+
expect(data).toHaveProperty('created_by', 'importer');
1089+
});
1090+
1091+
it('does NOT report when nothing was stripped', async () => {
1092+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
1093+
const events: any[] = [];
1094+
await engine.update('doc', { id: '1', title: 'clean' }, {
1095+
onFieldsDropped: (e: any) => events.push(e),
1096+
} as any);
1097+
expect(events).toEqual([]);
1098+
});
1099+
1100+
it('a throwing listener never breaks the write', async () => {
1101+
vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
1102+
await expect(
1103+
engine.update('doc', { id: '1', created_by: 'x' }, {
1104+
onFieldsDropped: () => { throw new Error('listener bug'); },
1105+
} as any),
1106+
).resolves.not.toThrow();
1107+
expect(vi.mocked(mockDriver.update)).toHaveBeenCalledTimes(1);
1108+
});
1109+
});
1110+
10051111
describe('Expand Related Records', () => {
10061112
beforeEach(async () => {
10071113
engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import {
88
EngineUpdateOptions,
99
EngineDeleteOptions,
1010
EngineAggregateOptions,
11-
EngineCountOptions
11+
EngineCountOptions,
12+
type DroppedFieldsEvent
1213
} from '@objectstack/spec/data';
14+
import type { WriteObservabilityOptions } from '@objectstack/spec/contracts';
1315
import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled } from '@objectstack/spec/data';
1416
import { ExecutionContext, ExecutionContextSchema } from '@objectstack/spec/kernel';
1517
import { IDataDriver, IDataEngine, Logger, createLogger, withTransientRetry, type RetryOptions } from '@objectstack/core';
@@ -2345,7 +2347,13 @@ export class ObjectQL implements IDataEngine {
23452347
* validation passes, so a doomed attempt no longer consumes a sequence value
23462348
* (no number-range gaps from a rejected batch).
23472349
*/
2348-
async insert(object: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any> {
2350+
// [#3407] `WriteObservabilityOptions.onFieldsDropped` is accepted for
2351+
// signature symmetry with `update()` but never fires here: INSERT is
2352+
// deliberately exempt from the readonly/readonlyWhen strips (a create may
2353+
// legitimately seed read-only columns), and the FLS write gate throws
2354+
// instead of stripping. If insert ever gains a silent strip, wire the
2355+
// listener at that strip site — do not let it go silent.
2356+
async insert(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise<any> {
23492357
object = this.resolveObjectName(object);
23502358
this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) });
23512359
this.assertWriteAllowed(object, 'insert');
@@ -2597,7 +2605,7 @@ export class ObjectQL implements IDataEngine {
25972605
return this.insert(object, rows, { ...(options ?? {}), __partialRowErrors: true } as any);
25982606
}
25992607

2600-
async update(object: string, data: any, options?: EngineUpdateOptions): Promise<any> {
2608+
async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise<any> {
26012609
object = this.resolveObjectName(object);
26022610
this.logger.debug('Update operation starting', { object });
26032611
this.assertWriteAllowed(object, 'update');
@@ -2650,6 +2658,33 @@ export class ObjectQL implements IDataEngine {
26502658
Object.keys((opCtx.data ?? {}) as Record<string, unknown>),
26512659
);
26522660

2661+
// [#3407] Structured strip observability. The readonly/readonlyWhen strips
2662+
// below are LEGAL semantics (the write still succeeds without the locked
2663+
// fields), but until now the only trace was a server-side logger warn — a
2664+
// caller that reports success per requested field (a flow's `update_record`
2665+
// step) saw a clean success while the DB value never changed. When the
2666+
// caller registers `onFieldsDropped`, report each strip pass's dropped
2667+
// keys back with its reason. Diffing before/after key sets is exact here:
2668+
// every strip helper returns the SAME reference when nothing was dropped,
2669+
// else a shallow copy with keys removed. A listener fault must never break
2670+
// the write.
2671+
const onFieldsDropped = options?.onFieldsDropped;
2672+
const reportDroppedFields = (
2673+
before: Record<string, unknown> | null | undefined,
2674+
after: Record<string, unknown> | null | undefined,
2675+
reason: DroppedFieldsEvent['reason'],
2676+
): void => {
2677+
if (!onFieldsDropped || before === after || !before) return;
2678+
const afterObj = (after ?? {}) as Record<string, unknown>;
2679+
const fields = Object.keys(before).filter((k) => !(k in afterObj));
2680+
if (fields.length === 0) return;
2681+
try {
2682+
onFieldsDropped({ object, fields, reason });
2683+
} catch (err) {
2684+
this.logger.warn('onFieldsDropped listener threw — ignored', { object, error: err });
2685+
}
2686+
};
2687+
26532688
await this.executeWithMiddleware(opCtx, async () => {
26542689
const hookContext: HookContext = {
26552690
object,
@@ -2688,14 +2723,18 @@ export class ObjectQL implements IDataEngine {
26882723
// B2: drop writes to fields locked by a TRUE `readonlyWhen` — the
26892724
// field is read-only for this record's state, so the incoming
26902725
// change is ignored (the persisted value is kept).
2691-
hookContext.input.data = stripReadonlyWhenFields(updateSchema as any, hookContext.input.data as Record<string, unknown>, priorRecord, this.logger) as any;
2726+
const preRoWhen = hookContext.input.data as Record<string, unknown>;
2727+
hookContext.input.data = stripReadonlyWhenFields(updateSchema as any, preRoWhen, priorRecord, this.logger) as any;
2728+
reportDroppedFields(preRoWhen, hookContext.input.data as Record<string, unknown>, 'readonly_when');
26922729
// [#2948] Enforce STATIC `readonly` on the write path for
26932730
// non-system callers (system writes legitimately set read-only
26942731
// columns and are exempt). Runs AFTER hooks/middleware stamped
26952732
// their columns; `suppliedKeys` ensures only caller-forged
26962733
// read-only writes are dropped, never the server stamps.
26972734
if (!opCtx.context?.isSystem) {
2698-
hookContext.input.data = stripReadonlyFields(updateSchema as any, hookContext.input.data as Record<string, unknown>, suppliedKeys, this.logger) as any;
2735+
const preRo = hookContext.input.data as Record<string, unknown>;
2736+
hookContext.input.data = stripReadonlyFields(updateSchema as any, preRo, suppliedKeys, this.logger) as any;
2737+
reportDroppedFields(preRo, hookContext.input.data as Record<string, unknown>, 'readonly');
26992738
}
27002739
evaluateValidationRules(updateSchema as any, hookContext.input.data as Record<string, unknown>, 'update', { previous: priorRecord, logger: this.logger, currentUser: this.buildEvalUser(opCtx.context) });
27012740
result = await driver.update(object, hookContext.input.id as string, hookContext.input.data as Record<string, unknown>, hookContext.input.options as any);
@@ -2739,14 +2778,18 @@ export class ObjectQL implements IDataEngine {
27392778
// `where` to reach the unlocked rows). Symmetric with the
27402779
// single-id `stripReadonlyWhenFields`; INSERT stays exempt.
27412780
if (payloadHasReadonlyWhen) {
2742-
hookContext.input.data = stripReadonlyWhenFieldsMulti(updateSchema as any, hookContext.input.data as Record<string, unknown>, priorRows, this.logger) as any;
2781+
const preRoWhenMulti = hookContext.input.data as Record<string, unknown>;
2782+
hookContext.input.data = stripReadonlyWhenFieldsMulti(updateSchema as any, preRoWhenMulti, priorRows, this.logger) as any;
2783+
reportDroppedFields(preRoWhenMulti, hookContext.input.data as Record<string, unknown>, 'readonly_when');
27432784
}
27442785
// [#2948] Same static-`readonly` write guard on the bulk path —
27452786
// a forged read-only column in a multi-row update is dropped for
27462787
// non-system callers (a foreign `organization_id` is additionally
27472788
// rejected upstream by the tenant write wall, #2946).
27482789
if (!opCtx.context?.isSystem) {
2749-
hookContext.input.data = stripReadonlyFields(updateSchema as any, hookContext.input.data as Record<string, unknown>, suppliedKeys, this.logger) as any;
2790+
const preRoMulti = hookContext.input.data as Record<string, unknown>;
2791+
hookContext.input.data = stripReadonlyFields(updateSchema as any, preRoMulti, suppliedKeys, this.logger) as any;
2792+
reportDroppedFields(preRoMulti, hookContext.input.data as Record<string, unknown>, 'readonly');
27502793
}
27512794
// [#3106] Same enforcement the single-id branch runs at its
27522795
// `evaluateValidationRules` call, applied per matched row: any

0 commit comments

Comments
 (0)