feat(automation): update_record/create_record 步骤对被静默剥离的写入字段挂 warning(#3407)

.changeset/update-record-dropped-field-warnings.md

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

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

@@ -37,9 +37,9 @@ export interface IDataEngine {
   count(objectName: string, query?: EngineCountOptions): Promise<number>;
   aggregate(objectName: string, query: EngineAggregateOptions): Promise<any[]>;
 
-  // Mutation
-  insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any>;
-  update(objectName: string, data: any, options?: EngineUpdateOptions): Promise<any>;
+  // Mutation (write ops also accept in-process WriteObservabilityOptions — see `update`)
+  insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise<any>;
+  update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise<any>;
   delete(objectName: string, options?: EngineDeleteOptions): Promise<any>;
 
   // AI / Vector Search (optional)
@@ -222,6 +222,36 @@ interface EngineUpdateOptions {
 }
 ```
 
+### WriteObservabilityOptions
+
+The write methods (`insert` / `update`) additionally accept an **in-process**
+`onFieldsDropped` listener. The engine invokes it when caller-supplied write
+fields are legally stripped from the payload before the driver write — static
+`readonly` fields or a TRUE `readonlyWhen` predicate. The write still succeeds;
+the listener exists so callers that report per-field success (e.g. the flow
+engine's `update_record` step) can surface a warning instead of a silent
+success.
+
+```typescript
+interface WriteObservabilityOptions {
+  onFieldsDropped?: (event: DroppedFieldsEvent) => void;
+}
+
+interface DroppedFieldsEvent {
+  object: string;                        // resolved object name
+  fields: string[];                      // caller-supplied fields that were dropped
+  reason: 'readonly' | 'readonly_when';  // why they were dropped
+}
+```
+
+<Callout type="warn">
+`onFieldsDropped` is a **TS-contract-level, in-process-only** channel. It is
+deliberately not part of the serializable Zod options schemas: a function is
+unrepresentable in JSON Schema and cannot cross the RPC (Virtual Data Engine)
+boundary, so remote callers never receive these events. A listener that throws
+never breaks the write — the engine catches and logs.
+</Callout>
+
 ### delete
 
 Deletes record(s) matching the `where` condition.

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

@@ -1,38 +1,50 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
-import { 
+import {
   EngineQueryOptions,
-  DataEngineInsertOptions, 
-  EngineUpdateOptions, 
+  DataEngineInsertOptions,
+  EngineUpdateOptions,
   EngineDeleteOptions,
-  EngineAggregateOptions, 
+  EngineAggregateOptions,
   EngineCountOptions,
   DataEngineRequest,
+  DroppedFieldsEvent,
 } from '@objectstack/spec/data';
 
+/**
+ * In-process write-observability hooks for `insert`/`update` (#3407).
+ * Mirror of `WriteObservabilityOptions` in `@objectstack/spec/contracts` —
+ * see that definition for the full rationale (in-process only; never part of
+ * the serializable options schemas or the RPC boundary).
+ */
+export interface WriteObservabilityOptions {
+  /** Called once per strip pass that dropped ≥1 caller-supplied field. */
+  onFieldsDropped?: (event: DroppedFieldsEvent) => void;
+}
+
 /**
  * IDataEngine - Standard Data Engine Interface
- * 
+ *
  * Abstract interface for data persistence capabilities.
  * Following the Dependency Inversion Principle - plugins depend on this interface,
  * not on concrete database implementations.
- * 
+ *
  * All query methods use standard QueryAST parameter names
  * (where/fields/orderBy/limit/offset/expand) to eliminate mechanical translation
  * between the Engine and Driver layers.
- * 
+ *
  * Aligned with 'src/data/data-engine.zod.ts' in @objectstack/spec.
  */
 
 export interface IDataEngine {
   find(objectName: string, query?: EngineQueryOptions): Promise<any[]>;
   findOne(objectName: string, query?: EngineQueryOptions): Promise<any>;
-  insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any>;
-  update(objectName: string, data: any, options?: EngineUpdateOptions): Promise<any>;
+  insert(objectName: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise<any>;
+  update(objectName: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise<any>;
   delete(objectName: string, options?: EngineDeleteOptions): Promise<any>;
   count(objectName: string, query?: EngineCountOptions): Promise<number>;
   aggregate(objectName: string, query: EngineAggregateOptions): Promise<any[]>;
-  
+
   /**
    * Vector Search (AI/RAG)
    */
@@ -48,5 +60,3 @@ export interface IDataEngine {
    */
   execute?(command: any, options?: Record<string, any>): Promise<any>;
 }
-
-

packages/objectql/src/engine.test.ts

@@ -1002,6 +1002,112 @@ describe('ObjectQL Engine', () => {
         });
     });
 
+    /**
+     * #3407 — the readonly/readonlyWhen strips are legal semantics, but they
+     * must not be SILENT to the caller: `options.onFieldsDropped` reports each
+     * strip pass's dropped keys + reason so callers (flow `update_record`
+     * steps) can surface a warning on an otherwise-successful write.
+     */
+    describe('Dropped-field write observability (#3407)', () => {
+        beforeEach(async () => {
+            engine.registerDriver(mockDriver, true);
+            await engine.init();
+            (mockDriver as any).updateMany = vi.fn().mockResolvedValue(1);
+        });
+
+        const docSchema = {
+            name: 'doc',
+            fields: {
+                title: { type: 'text' },
+                created_by: { type: 'text', readonly: true },
+            },
+        } as any;
+
+        it('reports caller-supplied static-readonly fields stripped on a single-id update', async () => {
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
+            const events: any[] = [];
+            await engine.update('doc', { id: '1', title: 'x', created_by: 'attacker' }, {
+                onFieldsDropped: (e: any) => events.push(e),
+            } as any);
+
+            expect(events).toEqual([{ object: 'doc', fields: ['created_by'], reason: 'readonly' }]);
+            // The strip behavior itself is unchanged: the write proceeded without the field.
+            const [, , data] = vi.mocked(mockDriver.update).mock.calls[0];
+            expect(data).not.toHaveProperty('created_by');
+            expect(data).toHaveProperty('title', 'x');
+        });
+
+        it('reports a readonlyWhen-locked field on a single-id update (reason readonly_when)', async () => {
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue({
+                name: 'invoice',
+                fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' } },
+            } as any);
+            vi.mocked(mockDriver.findOne).mockResolvedValue({ id: '1', locked: true, amount: 100 } as any);
+
+            const events: any[] = [];
+            await engine.update('invoice', { id: '1', amount: 999 }, {
+                onFieldsDropped: (e: any) => events.push(e),
+            } as any);
+
+            expect(events).toEqual([{ object: 'invoice', fields: ['amount'], reason: 'readonly_when' }]);
+            const [, , data] = vi.mocked(mockDriver.update).mock.calls[0];
+            expect(data).not.toHaveProperty('amount');
+        });
+
+        it('reports bulk-path strips too (multi update — locked in ≥1 matched row)', async () => {
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue({
+                name: 'invoice',
+                fields: { amount: { type: 'number', readonlyWhen: 'record.locked == true' } },
+            } as any);
+            vi.mocked(mockDriver.find).mockResolvedValue([
+                { id: 'a', locked: false, amount: 10 },
+                { id: 'b', locked: true, amount: 20 },
+            ] as any);
+
+            const events: any[] = [];
+            await engine.update('invoice', { amount: 999 }, {
+                where: { status: 'draft' }, multi: true,
+                onFieldsDropped: (e: any) => events.push(e),
+            } as any);
+
+            expect(events).toEqual([{ object: 'invoice', fields: ['amount'], reason: 'readonly_when' }]);
+            const [, , data] = (mockDriver as any).updateMany.mock.calls[0];
+            expect(data).not.toHaveProperty('amount');
+        });
+
+        it('does NOT report for a system-context update (the readonly strip is skipped)', async () => {
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
+            const events: any[] = [];
+            await engine.update('doc', { id: '1', created_by: 'importer' }, {
+                context: { isSystem: true },
+                onFieldsDropped: (e: any) => events.push(e),
+            } as any);
+            expect(events).toEqual([]);
+            // System writes legitimately set read-only columns — kept, not stripped.
+            const [, , data] = vi.mocked(mockDriver.update).mock.calls[0];
+            expect(data).toHaveProperty('created_by', 'importer');
+        });
+
+        it('does NOT report when nothing was stripped', async () => {
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
+            const events: any[] = [];
+            await engine.update('doc', { id: '1', title: 'clean' }, {
+                onFieldsDropped: (e: any) => events.push(e),
+            } as any);
+            expect(events).toEqual([]);
+        });
+
+        it('a throwing listener never breaks the write', async () => {
+            vi.mocked(SchemaRegistry.getObject).mockReturnValue(docSchema);
+            await expect(
+                engine.update('doc', { id: '1', created_by: 'x' }, {
+                    onFieldsDropped: () => { throw new Error('listener bug'); },
+                } as any),
+            ).resolves.not.toThrow();
+            expect(vi.mocked(mockDriver.update)).toHaveBeenCalledTimes(1);
+        });
+    });
+
     describe('Expand Related Records', () => {
         beforeEach(async () => {
             engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

@@ -8,8 +8,10 @@ import {
   EngineUpdateOptions,
   EngineDeleteOptions,
   EngineAggregateOptions,
-  EngineCountOptions
+  EngineCountOptions,
+  type DroppedFieldsEvent
 } from '@objectstack/spec/data';
+import type { WriteObservabilityOptions } from '@objectstack/spec/contracts';
 import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled } from '@objectstack/spec/data';
 import { ExecutionContext, ExecutionContextSchema } from '@objectstack/spec/kernel';
 import { IDataDriver, IDataEngine, Logger, createLogger, withTransientRetry, type RetryOptions } from '@objectstack/core';
@@ -2345,7 +2347,13 @@ export class ObjectQL implements IDataEngine {
    * validation passes, so a doomed attempt no longer consumes a sequence value
    * (no number-range gaps from a rejected batch).
    */
-  async insert(object: string, data: any | any[], options?: DataEngineInsertOptions): Promise<any> {
+  // [#3407] `WriteObservabilityOptions.onFieldsDropped` is accepted for
+  // signature symmetry with `update()` but never fires here: INSERT is
+  // deliberately exempt from the readonly/readonlyWhen strips (a create may
+  // legitimately seed read-only columns), and the FLS write gate throws
+  // instead of stripping. If insert ever gains a silent strip, wire the
+  // listener at that strip site — do not let it go silent.
+  async insert(object: string, data: any | any[], options?: DataEngineInsertOptions & WriteObservabilityOptions): Promise<any> {
     object = this.resolveObjectName(object);
     this.logger.debug('Insert operation starting', { object, isBatch: Array.isArray(data) });
     this.assertWriteAllowed(object, 'insert');
@@ -2597,7 +2605,7 @@ export class ObjectQL implements IDataEngine {
     return this.insert(object, rows, { ...(options ?? {}), __partialRowErrors: true } as any);
   }
 
-  async update(object: string, data: any, options?: EngineUpdateOptions): Promise<any> {
+  async update(object: string, data: any, options?: EngineUpdateOptions & WriteObservabilityOptions): Promise<any> {
      object = this.resolveObjectName(object);
      this.logger.debug('Update operation starting', { object });
      this.assertWriteAllowed(object, 'update');
@@ -2650,6 +2658,33 @@ export class ObjectQL implements IDataEngine {
        Object.keys((opCtx.data ?? {}) as Record<string, unknown>),
      );
 
+     // [#3407] Structured strip observability. The readonly/readonlyWhen strips
+     // below are LEGAL semantics (the write still succeeds without the locked
+     // fields), but until now the only trace was a server-side logger warn — a
+     // caller that reports success per requested field (a flow's `update_record`
+     // step) saw a clean success while the DB value never changed. When the
+     // caller registers `onFieldsDropped`, report each strip pass's dropped
+     // keys back with its reason. Diffing before/after key sets is exact here:
+     // every strip helper returns the SAME reference when nothing was dropped,
+     // else a shallow copy with keys removed. A listener fault must never break
+     // the write.
+     const onFieldsDropped = options?.onFieldsDropped;
+     const reportDroppedFields = (
+       before: Record<string, unknown> | null | undefined,
+       after: Record<string, unknown> | null | undefined,
+       reason: DroppedFieldsEvent['reason'],
+     ): void => {
+       if (!onFieldsDropped || before === after || !before) return;
+       const afterObj = (after ?? {}) as Record<string, unknown>;
+       const fields = Object.keys(before).filter((k) => !(k in afterObj));
+       if (fields.length === 0) return;
+       try {
+         onFieldsDropped({ object, fields, reason });
+       } catch (err) {
+         this.logger.warn('onFieldsDropped listener threw — ignored', { object, error: err });
+       }
+     };
+
      await this.executeWithMiddleware(opCtx, async () => {
        const hookContext: HookContext = {
           object,
@@ -2688,14 +2723,18 @@ export class ObjectQL implements IDataEngine {
                // B2: drop writes to fields locked by a TRUE `readonlyWhen` — the
                // field is read-only for this record's state, so the incoming
                // change is ignored (the persisted value is kept).
-               hookContext.input.data = stripReadonlyWhenFields(updateSchema as any, hookContext.input.data as Record<string, unknown>, priorRecord, this.logger) as any;
+               const preRoWhen = hookContext.input.data as Record<string, unknown>;
+               hookContext.input.data = stripReadonlyWhenFields(updateSchema as any, preRoWhen, priorRecord, this.logger) as any;
+               reportDroppedFields(preRoWhen, hookContext.input.data as Record<string, unknown>, 'readonly_when');
                // [#2948] Enforce STATIC `readonly` on the write path for
                // non-system callers (system writes legitimately set read-only
                // columns and are exempt). Runs AFTER hooks/middleware stamped
                // their columns; `suppliedKeys` ensures only caller-forged
                // read-only writes are dropped, never the server stamps.
                if (!opCtx.context?.isSystem) {
-                   hookContext.input.data = stripReadonlyFields(updateSchema as any, hookContext.input.data as Record<string, unknown>, suppliedKeys, this.logger) as any;
+                   const preRo = hookContext.input.data as Record<string, unknown>;
+                   hookContext.input.data = stripReadonlyFields(updateSchema as any, preRo, suppliedKeys, this.logger) as any;
+                   reportDroppedFields(preRo, hookContext.input.data as Record<string, unknown>, 'readonly');
                }
                evaluateValidationRules(updateSchema as any, hookContext.input.data as Record<string, unknown>, 'update', { previous: priorRecord, logger: this.logger, currentUser: this.buildEvalUser(opCtx.context) });
                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 {
                // `where` to reach the unlocked rows). Symmetric with the
                // single-id `stripReadonlyWhenFields`; INSERT stays exempt.
                if (payloadHasReadonlyWhen) {
-                   hookContext.input.data = stripReadonlyWhenFieldsMulti(updateSchema as any, hookContext.input.data as Record<string, unknown>, priorRows, this.logger) as any;
+                   const preRoWhenMulti = hookContext.input.data as Record<string, unknown>;
+                   hookContext.input.data = stripReadonlyWhenFieldsMulti(updateSchema as any, preRoWhenMulti, priorRows, this.logger) as any;
+                   reportDroppedFields(preRoWhenMulti, hookContext.input.data as Record<string, unknown>, 'readonly_when');
                }
                // [#2948] Same static-`readonly` write guard on the bulk path —
                // a forged read-only column in a multi-row update is dropped for
                // non-system callers (a foreign `organization_id` is additionally
                // rejected upstream by the tenant write wall, #2946).
                if (!opCtx.context?.isSystem) {
-                   hookContext.input.data = stripReadonlyFields(updateSchema as any, hookContext.input.data as Record<string, unknown>, suppliedKeys, this.logger) as any;
+                   const preRoMulti = hookContext.input.data as Record<string, unknown>;
+                   hookContext.input.data = stripReadonlyFields(updateSchema as any, preRoMulti, suppliedKeys, this.logger) as any;
+                   reportDroppedFields(preRoMulti, hookContext.input.data as Record<string, unknown>, 'readonly');
                }
                // [#3106] Same enforcement the single-id branch runs at its
                // `evaluateValidationRules` call, applied per matched row: any