diff --git a/.changeset/rest-update-dropped-fields.md b/.changeset/rest-update-dropped-fields.md new file mode 100644 index 0000000000..23b9bf345e --- /dev/null +++ b/.changeset/rest-update-dropped-fields.md @@ -0,0 +1,29 @@ +--- +"@objectstack/spec": minor +"@objectstack/metadata-protocol": minor +--- + +feat(protocol): surface silently-stripped write fields on the update response (#3431) + +`PATCH /data/:object/:id` returned `200 + record` even when the data layer +legally stripped some of the caller's write — a static `readonly` field +(#2948) or one locked by a TRUE `readonlyWhen` predicate (#3042). The strip is +correct semantics, but it was **silent** on this surface: the only signal lived +in a server-side WARN log, so an API client's partial write "just didn't save" +with no way to detect it. This is the same gap #3407/#3413 closed for the flow +engine, one surface over — REST — reusing the engine's existing +`onFieldsDropped` observability channel. + +- **spec** — `UpdateDataResponseSchema` gains an optional `droppedFields`: + `DroppedFieldsEvent[]` (`{ object, fields, reason: 'readonly' | 'readonly_when' }`), + present only when ≥1 field was dropped. Purely additive — existing clients + reading `.record` are unaffected; `success` semantics are unchanged (the + strip is surfaced, not escalated to a failure). +- **metadata-protocol** — `updateData` threads an `onFieldsDropped` collector + into `engine.update` and attaches the collected events to its response + envelope, so the REST PATCH handler's `res.json(result)` carries them. + +Not wired on `createData`/POST: `insert` is readonly-exempt by design (INSERT +may set read-only columns), so it produces no dropped events — wiring it would +be inert. If insert ever gains a silent strip, wire it then. Bulk / batch / +GraphQL write paths remain follow-ups. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index 3a0224acb5..1188ed4dfb 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -1179,6 +1179,7 @@ const result = AiInsightsRequest.parse(data); | **object** | `string` | ✅ | Object name | | **id** | `string` | ✅ | Updated record ID | | **record** | `Record` | ✅ | Updated record | +| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when'> }[]` | optional | Advisory: caller-supplied fields the data layer legally stripped before persistence — static `readonly` (#2948) or a TRUE `readonlyWhen` predicate (#3042). Present only when ≥1 field was dropped. The update still succeeds (the strip is legitimate semantics), so this is the only signal that part of the write did not land — the strip was previously silent on this surface (#3431). | --- diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index fce493ebe6..28f1e9ec76 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -17,6 +17,7 @@ import type { import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api'; import { readServiceSelfInfo } from '@objectstack/spec/api'; import { parseFilterAST, isFilterAST } from '@objectstack/spec/data'; +import type { DroppedFieldsEvent } from '@objectstack/spec/data'; import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared'; import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui'; import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system'; @@ -2928,11 +2929,20 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context); const opts: any = { where: { id: request.id } }; if (request.context !== undefined) opts.context = request.context; + // [#3431] Capture fields the data layer legally strips (static `readonly` + // #2948 / conditional `readonlyWhen` #3042) so the caller learns which of + // its writes didn't land. The PATCH response was previously `200 + record` + // with no signal a field was dropped — the same silent-strip gap #3407 + // closed for the flow engine, one surface over. `success` is unchanged; + // the strip is legitimate semantics, surfaced not escalated. + const dropped: DroppedFieldsEvent[] = []; + opts.onFieldsDropped = (event: DroppedFieldsEvent) => { dropped.push(event); }; const result = await this.engine.update(request.object, request.data, opts); return { object: request.object, id: request.id, - record: result + record: result, + ...(dropped.length > 0 ? { droppedFields: dropped } : {}), }; } diff --git a/packages/objectql/src/protocol-data.test.ts b/packages/objectql/src/protocol-data.test.ts index 2df6b194b9..7d4f8b44f1 100644 --- a/packages/objectql/src/protocol-data.test.ts +++ b/packages/objectql/src/protocol-data.test.ts @@ -467,6 +467,64 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => { }); }); + // ═══════════════════════════════════════════════════════════════ + // Dropped-field observability (#3431) — updateData surfaces the + // fields the engine legally strips (readonly / readonlyWhen) on its + // response, so the PATCH caller learns a write didn't land instead + // of receiving a silent 200 + record. + // ═══════════════════════════════════════════════════════════════ + describe('dropped-field observability (#3431)', () => { + beforeEach(() => { + mockEngine.update = vi.fn().mockResolvedValue({ id: 'r1', updated_at: '2026-05-22T07:14:33.000Z' }); + }); + + it('passes an onFieldsDropped listener into the engine update options', async () => { + await protocol.updateData({ object: 'task', id: 'r1', data: { name: 'New' } }); + const opts = mockEngine.update.mock.calls[0][2]; + expect(typeof opts.onFieldsDropped).toBe('function'); + }); + + it('surfaces engine-dropped fields on the response, keeping the record', async () => { + // The engine strips a readonly field and reports it via the listener. + mockEngine.update.mockImplementation(async (_obj: string, _data: any, opts: any) => { + opts?.onFieldsDropped?.({ object: 'crm_opportunity', fields: ['approval_status'], reason: 'readonly' }); + return { id: 'r1', updated_at: '2026-05-22T07:14:33.000Z' }; + }); + + const res: any = await protocol.updateData({ + object: 'crm_opportunity', + id: 'r1', + data: { approval_status: 'approved', notes: 'ok' }, + }); + + // The write still succeeds (record returned) AND the dropped field is + // reported structurally — no longer silent on this surface. + expect(res.record).toMatchObject({ id: 'r1' }); + expect(res.droppedFields).toEqual([ + { object: 'crm_opportunity', fields: ['approval_status'], reason: 'readonly' }, + ]); + }); + + it('collects multiple strip passes (readonly + readonlyWhen)', async () => { + mockEngine.update.mockImplementation(async (_obj: string, _data: any, opts: any) => { + opts?.onFieldsDropped?.({ object: 'crm_case', fields: ['locked_at'], reason: 'readonly_when' }); + opts?.onFieldsDropped?.({ object: 'crm_case', fields: ['created_by'], reason: 'readonly' }); + return { id: 'r1' }; + }); + + const res: any = await protocol.updateData({ object: 'crm_case', id: 'r1', data: {} }); + + expect(res.droppedFields).toHaveLength(2); + expect(res.droppedFields.map((e: any) => e.reason)).toEqual(['readonly_when', 'readonly']); + }); + + it('omits droppedFields entirely when nothing was stripped', async () => { + const res: any = await protocol.updateData({ object: 'task', id: 'r1', data: { name: 'New' } }); + expect(res.record).toMatchObject({ id: 'r1' }); + expect(res).not.toHaveProperty('droppedFields'); + }); + }); + // ═══════════════════════════════════════════════════════════════ // cloneData — duplicate a record, gated by enable.clone // ═══════════════════════════════════════════════════════════════ diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index d814f43d8c..e22bc8cbb1 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -6,6 +6,7 @@ import { DiscoverySchema } from './discovery.zod'; import { BatchUpdateRequestSchema, BatchUpdateResponseSchema, BatchOptionsSchema } from './batch.zod'; import { MetadataCacheRequestSchema, MetadataCacheResponseSchema } from './http-cache.zod'; import { QuerySchema } from '../data/query.zod'; +import { DroppedFieldsEventSchema } from '../data/data-engine.zod'; import { AnalyticsQueryRequestSchema, AnalyticsResultResponseSchema, @@ -426,6 +427,14 @@ export const UpdateDataResponseSchema = lazySchema(() => z.object({ object: z.string().describe('Object name'), id: z.string().describe('Updated record ID'), record: z.record(z.string(), z.unknown()).describe('Updated record'), + droppedFields: z.array(DroppedFieldsEventSchema).optional().describe( + 'Advisory: caller-supplied fields the data layer legally stripped before ' + + 'persistence — static `readonly` (#2948) or a TRUE `readonlyWhen` predicate ' + + '(#3042). Present only when ≥1 field was dropped. The update still succeeds ' + + '(the strip is legitimate semantics), so this is the only signal that part ' + + 'of the write did not land — the strip was previously silent on this ' + + 'surface (#3431).' + ), })); /**