Skip to content

Commit 38b2ba3

Browse files
committed
feat(protocol): surface silently-stripped write fields on the update response (#3431)
PATCH /data returned 200 + record even when the data layer legally stripped part of the caller's write (static readonly #2948 / readonlyWhen #3042). The strip is correct semantics but was silent on this surface — the only signal lived in a server WARN log. This reuses the engine's onFieldsDropped channel (#3407/#3413) to report it, one surface over. - spec: UpdateDataResponseSchema gains optional droppedFields (DroppedFieldsEvent[]), present only when >=1 field dropped; purely additive, success unchanged - metadata-protocol: updateData threads onFieldsDropped and attaches collected events to its response envelope, which the REST PATCH handler passes through createData/POST is not wired: insert is readonly-exempt so produces no events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HaofCZbsPTHE2oJedvKd77
1 parent af5a224 commit 38b2ba3

5 files changed

Lines changed: 108 additions & 1 deletion

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata-protocol": minor
4+
---
5+
6+
feat(protocol): surface silently-stripped write fields on the update response (#3431)
7+
8+
`PATCH /data/:object/:id` returned `200 + record` even when the data layer
9+
legally stripped some of the caller's write — a static `readonly` field
10+
(#2948) or one locked by a TRUE `readonlyWhen` predicate (#3042). The strip is
11+
correct semantics, but it was **silent** on this surface: the only signal lived
12+
in a server-side WARN log, so an API client's partial write "just didn't save"
13+
with no way to detect it. This is the same gap #3407/#3413 closed for the flow
14+
engine, one surface over — REST — reusing the engine's existing
15+
`onFieldsDropped` observability channel.
16+
17+
- **spec**`UpdateDataResponseSchema` gains an optional `droppedFields`:
18+
`DroppedFieldsEvent[]` (`{ object, fields, reason: 'readonly' | 'readonly_when' }`),
19+
present only when ≥1 field was dropped. Purely additive — existing clients
20+
reading `.record` are unaffected; `success` semantics are unchanged (the
21+
strip is surfaced, not escalated to a failure).
22+
- **metadata-protocol**`updateData` threads an `onFieldsDropped` collector
23+
into `engine.update` and attaches the collected events to its response
24+
envelope, so the REST PATCH handler's `res.json(result)` carries them.
25+
26+
Not wired on `createData`/POST: `insert` is readonly-exempt by design (INSERT
27+
may set read-only columns), so it produces no dropped events — wiring it would
28+
be inert. If insert ever gains a silent strip, wire it then. Bulk / batch /
29+
GraphQL write paths remain follow-ups.

content/docs/references/api/protocol.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,7 @@ const result = AiInsightsRequest.parse(data);
11791179
| **object** | `string` || Object name |
11801180
| **id** | `string` || Updated record ID |
11811181
| **record** | `Record<string, any>` || Updated record |
1182+
| **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). |
11821183

11831184

11841185
---

packages/metadata-protocol/src/protocol.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {
1717
import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
1818
import { readServiceSelfInfo } from '@objectstack/spec/api';
1919
import { parseFilterAST, isFilterAST } from '@objectstack/spec/data';
20+
import type { DroppedFieldsEvent } from '@objectstack/spec/data';
2021
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
2122
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
2223
import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system';
@@ -2928,11 +2929,20 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
29282929
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
29292930
const opts: any = { where: { id: request.id } };
29302931
if (request.context !== undefined) opts.context = request.context;
2932+
// [#3431] Capture fields the data layer legally strips (static `readonly`
2933+
// #2948 / conditional `readonlyWhen` #3042) so the caller learns which of
2934+
// its writes didn't land. The PATCH response was previously `200 + record`
2935+
// with no signal a field was dropped — the same silent-strip gap #3407
2936+
// closed for the flow engine, one surface over. `success` is unchanged;
2937+
// the strip is legitimate semantics, surfaced not escalated.
2938+
const dropped: DroppedFieldsEvent[] = [];
2939+
opts.onFieldsDropped = (event: DroppedFieldsEvent) => { dropped.push(event); };
29312940
const result = await this.engine.update(request.object, request.data, opts);
29322941
return {
29332942
object: request.object,
29342943
id: request.id,
2935-
record: result
2944+
record: result,
2945+
...(dropped.length > 0 ? { droppedFields: dropped } : {}),
29362946
};
29372947
}
29382948

packages/objectql/src/protocol-data.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,64 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => {
467467
});
468468
});
469469

470+
// ═══════════════════════════════════════════════════════════════
471+
// Dropped-field observability (#3431) — updateData surfaces the
472+
// fields the engine legally strips (readonly / readonlyWhen) on its
473+
// response, so the PATCH caller learns a write didn't land instead
474+
// of receiving a silent 200 + record.
475+
// ═══════════════════════════════════════════════════════════════
476+
describe('dropped-field observability (#3431)', () => {
477+
beforeEach(() => {
478+
mockEngine.update = vi.fn().mockResolvedValue({ id: 'r1', updated_at: '2026-05-22T07:14:33.000Z' });
479+
});
480+
481+
it('passes an onFieldsDropped listener into the engine update options', async () => {
482+
await protocol.updateData({ object: 'task', id: 'r1', data: { name: 'New' } });
483+
const opts = mockEngine.update.mock.calls[0][2];
484+
expect(typeof opts.onFieldsDropped).toBe('function');
485+
});
486+
487+
it('surfaces engine-dropped fields on the response, keeping the record', async () => {
488+
// The engine strips a readonly field and reports it via the listener.
489+
mockEngine.update.mockImplementation(async (_obj: string, _data: any, opts: any) => {
490+
opts?.onFieldsDropped?.({ object: 'crm_opportunity', fields: ['approval_status'], reason: 'readonly' });
491+
return { id: 'r1', updated_at: '2026-05-22T07:14:33.000Z' };
492+
});
493+
494+
const res: any = await protocol.updateData({
495+
object: 'crm_opportunity',
496+
id: 'r1',
497+
data: { approval_status: 'approved', notes: 'ok' },
498+
});
499+
500+
// The write still succeeds (record returned) AND the dropped field is
501+
// reported structurally — no longer silent on this surface.
502+
expect(res.record).toMatchObject({ id: 'r1' });
503+
expect(res.droppedFields).toEqual([
504+
{ object: 'crm_opportunity', fields: ['approval_status'], reason: 'readonly' },
505+
]);
506+
});
507+
508+
it('collects multiple strip passes (readonly + readonlyWhen)', async () => {
509+
mockEngine.update.mockImplementation(async (_obj: string, _data: any, opts: any) => {
510+
opts?.onFieldsDropped?.({ object: 'crm_case', fields: ['locked_at'], reason: 'readonly_when' });
511+
opts?.onFieldsDropped?.({ object: 'crm_case', fields: ['created_by'], reason: 'readonly' });
512+
return { id: 'r1' };
513+
});
514+
515+
const res: any = await protocol.updateData({ object: 'crm_case', id: 'r1', data: {} });
516+
517+
expect(res.droppedFields).toHaveLength(2);
518+
expect(res.droppedFields.map((e: any) => e.reason)).toEqual(['readonly_when', 'readonly']);
519+
});
520+
521+
it('omits droppedFields entirely when nothing was stripped', async () => {
522+
const res: any = await protocol.updateData({ object: 'task', id: 'r1', data: { name: 'New' } });
523+
expect(res.record).toMatchObject({ id: 'r1' });
524+
expect(res).not.toHaveProperty('droppedFields');
525+
});
526+
});
527+
470528
// ═══════════════════════════════════════════════════════════════
471529
// cloneData — duplicate a record, gated by enable.clone
472530
// ═══════════════════════════════════════════════════════════════

packages/spec/src/api/protocol.zod.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { DiscoverySchema } from './discovery.zod';
66
import { BatchUpdateRequestSchema, BatchUpdateResponseSchema, BatchOptionsSchema } from './batch.zod';
77
import { MetadataCacheRequestSchema, MetadataCacheResponseSchema } from './http-cache.zod';
88
import { QuerySchema } from '../data/query.zod';
9+
import { DroppedFieldsEventSchema } from '../data/data-engine.zod';
910
import {
1011
AnalyticsQueryRequestSchema,
1112
AnalyticsResultResponseSchema,
@@ -426,6 +427,14 @@ export const UpdateDataResponseSchema = lazySchema(() => z.object({
426427
object: z.string().describe('Object name'),
427428
id: z.string().describe('Updated record ID'),
428429
record: z.record(z.string(), z.unknown()).describe('Updated record'),
430+
droppedFields: z.array(DroppedFieldsEventSchema).optional().describe(
431+
'Advisory: caller-supplied fields the data layer legally stripped before ' +
432+
'persistence — static `readonly` (#2948) or a TRUE `readonlyWhen` predicate ' +
433+
'(#3042). Present only when ≥1 field was dropped. The update still succeeds ' +
434+
'(the strip is legitimate semantics), so this is the only signal that part ' +
435+
'of the write did not land — the strip was previously silent on this ' +
436+
'surface (#3431).'
437+
),
429438
}));
430439

431440
/**

0 commit comments

Comments
 (0)