Skip to content

Commit 91f01c5

Browse files
claudeclaude
committed
feat(rest): surface silently-dropped write fields on PATCH/POST /data (#3431)
Wire the engine's `onFieldsDropped` strip-observability channel (#3413) through the DataProtocol and REST write path so an external API caller is no longer silently stripped when `readonly` (#2948) / `readonlyWhen` (#3042) drops caller-supplied fields. The write still succeeds — this only makes the strip observable (the same silent-success class #3407 fixed flow-side). - metadata-protocol: `updateData` collects the engine's `onFieldsDropped` events; `createData` surfaces the #3043 static-`readonly` INGRESS strip via a payload diff (that strip runs BEFORE the engine, which is INSERT-readonly- exempt, so the engine listener never sees it). Both attach an optional `droppedFields` list to the response when ≥1 field was dropped. - spec: `UpdateDataResponseSchema` / `CreateDataResponseSchema` gain an optional `droppedFields: DroppedFieldsEvent[]` — present only on a drop, so the shape stays backward-compatible for clients that only read `record`. - rest: PATCH `/data/:object/:id` and POST `/data/:object` echo drops as the `X-ObjectStack-Dropped-Fields` response header and keep the structured list on the body. Status/success semantics unchanged (200 update / 201 create). Tests: protocol passthrough (update forwards engine events; create surfaces the ingress strip; no-drop omits the key) and REST header/body (single + multi field, no-drop, create 201). Deferred (issue #3431 D2 open questions): bulk (`updateManyData` / `createManyData` / `batchData`) and GraphQL mutation wiring, typed `@objectstack/client` warnings, and adding the header to the Hono CORS `exposeHeaders` allow-list for cross-origin browser reads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNqeAmxCCq9jgLFwa9subq
1 parent 2ba560a commit 91f01c5

6 files changed

Lines changed: 396 additions & 8 deletions

File tree

‎.changeset/rest-patch-data-dropped-fields.md‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata-protocol": minor
4+
"@objectstack/rest": minor
5+
---
6+
7+
feat(rest): surface silently-dropped write fields on PATCH/POST /data (#3431)
8+
9+
#3413 (closes #3407) built the engine-level strip-observability channel
10+
(`WriteObservabilityOptions.onFieldsDropped`) and wired the flow side
11+
(`update_record` / `create_record` emit a step warning + `droppedFields`). The
12+
**REST write path was never wired**, so an external API caller writing N fields
13+
still got a bare `200 + record` when `readonly` (#2948) / `readonlyWhen` (#3042)
14+
stripping meant `< N` actually landed — the same silent-success class #3407
15+
fixed flow-side, just on HTTP. The only way to notice was a per-field diff of
16+
the returned row (which need not echo every field). This wires the channel
17+
through the protocol → REST, on both write verbs.
18+
19+
**Passthrough (metadata-protocol).** `updateData` now registers an
20+
`onFieldsDropped` collector on `engine.update` and returns the events on the
21+
response as `droppedFields`. `createData` surfaces the #3043 static-`readonly`
22+
INGRESS strip too — that strip runs at the protocol ingress
23+
(`stripReadonlyForInsert`), *before* the engine, so it is recovered by diffing
24+
the supplied payload against the stripped one (the engine's `onFieldsDropped` is
25+
also wired for a future insert-side engine strip). A faulty listener never
26+
breaks the write — the engine catches and logs.
27+
28+
**Contract (spec).** `UpdateDataResponseSchema` / `CreateDataResponseSchema`
29+
gain an **optional** `droppedFields: DroppedFieldsEvent[]` — present only when
30+
≥1 field was dropped. Optional + omit-when-empty keeps the response shape
31+
backward-compatible for clients that only read `record`.
32+
33+
**REST surface.** PATCH `/data/:object/:id` and POST `/data/:object` echo the
34+
drops as an `X-ObjectStack-Dropped-Fields` response header
35+
(`field;reason=<reason>` tokens, comma-joined — e.g.
36+
`approval_status;reason=readonly`) and keep the structured `droppedFields` on
37+
the body. **Status/success semantics are unchanged** (200 update / 201 create) —
38+
a strip is legitimate semantics, not a failure (same principle as #3413). The
39+
FLS write gate is untouched (it already fails closed with 403).
40+
41+
Out of scope (issue #3431 D2 open questions, deferred): bulk
42+
(`updateManyData` / `createManyData` / `batchData`) and GraphQL mutation wiring,
43+
typed `@objectstack/client` warnings, and adding the header to the Hono CORS
44+
`exposeHeaders` allow-list for cross-origin browser reads (the body
45+
`droppedFields` is the cross-origin-safe channel meanwhile).

‎packages/metadata-protocol/src/protocol.dropped-fields.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#3431] REST/API write paths must not SILENTLY drop caller-supplied fields.
4+
// #3413 built the engine-level `onFieldsDropped` channel and wired the flow
5+
// (`update_record`) side; this locks the DataProtocol passthrough that carries
6+
// the strip back to the REST layer:
7+
// - updateData forwards the engine's onFieldsDropped events (readonly /
8+
// readonly_when) onto the response as `droppedFields`;
9+
// - createData surfaces the #3043 static-`readonly` INGRESS strip, which runs
10+
// BEFORE the engine (so it is recovered by diffing the payload, not via the
11+
// engine listener) — symmetric with update;
12+
// - no strip → NO `droppedFields` key, so the response shape stays
13+
// backward-compatible for clients that only read `record`.
14+
15+
import { describe, it, expect, vi } from 'vitest';
16+
import { ObjectStackProtocolImplementation } from './protocol.js';
17+
18+
const SCHEMA = {
19+
name: 'approval_case',
20+
fields: {
21+
title: { name: 'title', type: 'text' },
22+
approval_status: { name: 'approval_status', type: 'text', readonly: true, defaultValue: 'draft' },
23+
},
24+
};
25+
26+
describe('updateData — forwards engine write strips as droppedFields (#3431)', () => {
27+
it('surfaces a readonly strip the engine reports via onFieldsDropped', async () => {
28+
const engine = {
29+
registry: { getObject: () => SCHEMA },
30+
// Stand in for the engine stripping `approval_status` and reporting it.
31+
update: vi.fn(async (object: string, data: any, options?: any) => {
32+
options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' });
33+
return { id: 'rec-1', title: data.title };
34+
}),
35+
findOne: vi.fn(async () => null),
36+
};
37+
const p = new ObjectStackProtocolImplementation(engine as any);
38+
const res: any = await p.updateData({
39+
object: 'approval_case',
40+
id: 'rec-1',
41+
data: { title: 'B', approval_status: 'approved' },
42+
context: { userId: 'u1' },
43+
});
44+
expect(res.droppedFields).toEqual([
45+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
46+
]);
47+
// The write still succeeded; the returned record is unchanged in shape.
48+
expect(res.record).toEqual({ id: 'rec-1', title: 'B' });
49+
});
50+
51+
it('forwards multiple strip passes in order (readonly_when then readonly)', async () => {
52+
const engine = {
53+
registry: { getObject: () => SCHEMA },
54+
update: vi.fn(async (object: string, _data: any, options?: any) => {
55+
options?.onFieldsDropped?.({ object, fields: ['locked'], reason: 'readonly_when' });
56+
options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' });
57+
return { id: 'rec-1' };
58+
}),
59+
findOne: vi.fn(async () => null),
60+
};
61+
const p = new ObjectStackProtocolImplementation(engine as any);
62+
const res: any = await p.updateData({ object: 'approval_case', id: 'rec-1', data: {} });
63+
expect(res.droppedFields).toEqual([
64+
{ object: 'approval_case', fields: ['locked'], reason: 'readonly_when' },
65+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
66+
]);
67+
});
68+
69+
it('omits droppedFields entirely when the engine stripped nothing', async () => {
70+
const engine = {
71+
registry: { getObject: () => SCHEMA },
72+
update: vi.fn(async (_o: string, data: any) => ({ id: 'rec-1', ...data })),
73+
findOne: vi.fn(async () => null),
74+
};
75+
const p = new ObjectStackProtocolImplementation(engine as any);
76+
const res: any = await p.updateData({ object: 'approval_case', id: 'rec-1', data: { title: 'B' } });
77+
expect(res).not.toHaveProperty('droppedFields');
78+
});
79+
});
80+
81+
describe('createData — surfaces the #3043 ingress readonly strip as droppedFields (#3431)', () => {
82+
function makeProtocol() {
83+
const engine = {
84+
registry: { getObject: (n: string) => (n === 'approval_case' ? SCHEMA : undefined) },
85+
insert: vi.fn(async (_object: string, data: any) => ({ id: 'rec-1', ...data })),
86+
};
87+
return { p: new ObjectStackProtocolImplementation(engine as any), engine };
88+
}
89+
90+
it('reports the forged readonly field a non-system create dropped', async () => {
91+
const { p } = makeProtocol();
92+
const res: any = await p.createData({
93+
object: 'approval_case',
94+
data: { title: 'A', approval_status: 'approved' },
95+
context: { userId: 'u1' },
96+
});
97+
expect(res.droppedFields).toEqual([
98+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
99+
]);
100+
// Stripped field is absent from the persisted payload (existing #3043 behaviour).
101+
expect(res.record).not.toHaveProperty('approval_status');
102+
});
103+
104+
it('omits droppedFields when the create seeds no readonly field', async () => {
105+
const { p } = makeProtocol();
106+
const res: any = await p.createData({
107+
object: 'approval_case',
108+
data: { title: 'A' },
109+
context: { userId: 'u1' },
110+
});
111+
expect(res).not.toHaveProperty('droppedFields');
112+
});
113+
114+
it('a system-context create keeps the field — no strip, no droppedFields', async () => {
115+
const { p } = makeProtocol();
116+
const res: any = await p.createData({
117+
object: 'approval_case',
118+
data: { title: 'A', approval_status: 'approved' },
119+
context: { isSystem: true },
120+
});
121+
expect(res).not.toHaveProperty('droppedFields');
122+
expect(res.record.approval_status).toBe('approved');
123+
});
124+
});

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type {
1616
} from '@objectstack/spec/api';
1717
import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
1818
import { readServiceSelfInfo } from '@objectstack/spec/api';
19-
import { parseFilterAST, isFilterAST } from '@objectstack/spec/data';
19+
import { parseFilterAST, isFilterAST, type DroppedFieldsEvent } from '@objectstack/spec/data';
2020
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
2121
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
2222
import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system';
@@ -588,6 +588,34 @@ function stripReadonlyForInsert(schema: any, data: any, context: any): any {
588588
return Array.isArray(data) ? data.map(stripRow) : stripRow(data);
589589
}
590590

591+
/**
592+
* [#3431] Recover a `DroppedFieldsEvent` from a before/after write-payload diff.
593+
*
594+
* The UPDATE strips (static `readonly` / `readonlyWhen`) run INSIDE the engine,
595+
* which reports them via the `onFieldsDropped` listener (wired in `updateData`).
596+
* The CREATE `readonly` strip, however, runs at THIS protocol ingress
597+
* (`stripReadonlyForInsert`, #3043) — BEFORE the engine — so the engine listener
598+
* never sees it. Diffing the caller-supplied keys against the stripped payload
599+
* recovers exactly which supplied fields the ingress strip removed, so the create
600+
* path can surface them symmetrically with update.
601+
*
602+
* Returns `null` when nothing was dropped (same reference, non-object, array, or
603+
* no key delta) so callers can `if (ev) dropped.push(ev)` without emitting empty
604+
* events. Mirrors the engine's own before/after key-set diff (`reportDroppedFields`
605+
* in objectql/engine.ts) so both channels agree on what "dropped" means.
606+
*/
607+
function diffDroppedFields(
608+
object: string,
609+
before: unknown,
610+
after: unknown,
611+
reason: DroppedFieldsEvent['reason'],
612+
): DroppedFieldsEvent | null {
613+
if (before === after || before == null || typeof before !== 'object' || Array.isArray(before)) return null;
614+
const afterObj = (after ?? {}) as Record<string, unknown>;
615+
const fields = Object.keys(before as Record<string, unknown>).filter((k) => !(k in afterObj));
616+
return fields.length > 0 ? { object, fields, reason } : null;
617+
}
618+
591619
/**
592620
* Service Configuration for Discovery
593621
* Maps service names to their routes and plugin providers.
@@ -2829,15 +2857,24 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
28292857
request.data,
28302858
request.context,
28312859
);
2832-
const result = await this.engine.insert(
2833-
request.object,
2834-
data,
2835-
request.context !== undefined ? { context: request.context } as any : undefined,
2836-
);
2860+
// [#3431] The #3043 ingress strip above is SILENT by contract; surface it
2861+
// so a REST/API caller learns which supplied fields were dropped, symmetric
2862+
// with `updateData`. The strip lives at THIS ingress (not the engine, which
2863+
// is INSERT-readonly-exempt, #3413), so recover it by diffing the supplied
2864+
// payload against the stripped one. The engine's `onFieldsDropped` is ALSO
2865+
// wired below so a FUTURE insert-side engine strip surfaces automatically
2866+
// through the same list instead of going silent.
2867+
const dropped: DroppedFieldsEvent[] = [];
2868+
const ingressDropped = diffDroppedFields(request.object, request.data, data, 'readonly');
2869+
if (ingressDropped) dropped.push(ingressDropped);
2870+
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
2871+
if (request.context !== undefined) opts.context = request.context;
2872+
const result = await this.engine.insert(request.object, data, opts);
28372873
return {
28382874
object: request.object,
28392875
id: result.id,
2840-
record: result
2876+
record: result,
2877+
...(dropped.length > 0 ? { droppedFields: dropped } : {}),
28412878
};
28422879
}
28432880

@@ -2928,11 +2965,20 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
29282965
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
29292966
const opts: any = { where: { id: request.id } };
29302967
if (request.context !== undefined) opts.context = request.context;
2968+
// [#3407/#3431] Capture the engine's LEGAL write strips (static `readonly`
2969+
// (#2948) / TRUE `readonlyWhen` (#3042)) so a REST/API caller is not left
2970+
// to field-by-field diff the returned row to discover a value never
2971+
// landed. The write still succeeds — this only makes the strip observable,
2972+
// mirroring service-automation's `update_record` wiring (#3413). A faulty
2973+
// listener never breaks the write (the engine catches + logs).
2974+
const dropped: DroppedFieldsEvent[] = [];
2975+
opts.onFieldsDropped = (e: DroppedFieldsEvent) => { dropped.push(e); };
29312976
const result = await this.engine.update(request.object, request.data, opts);
29322977
return {
29332978
object: request.object,
29342979
id: request.id,
2935-
record: result
2980+
record: result,
2981+
...(dropped.length > 0 ? { droppedFields: dropped } : {}),
29362982
};
29372983
}
29382984

‎packages/rest/src/rest-dropped-fields.test.ts‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#3431] The REST write paths must surface the engine's LEGAL field strips so
4+
// an external caller is not left to diff the returned row to notice a field
5+
// never landed (the same silent-success class #3407 fixed flow-side). The
6+
// DataProtocol result carries `droppedFields`; the REST layer echoes it as the
7+
// `X-ObjectStack-Dropped-Fields` response header AND keeps the structured list
8+
// on the body. The STATUS CODE is unchanged (200 update / 201 create) — a strip
9+
// is legitimate semantics, not a failure.
10+
11+
import { describe, it, expect, vi } from 'vitest';
12+
import { RestServer } from './rest-server';
13+
14+
function mockServer() {
15+
return {
16+
get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
17+
use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
18+
};
19+
}
20+
21+
function mockRes() {
22+
const res: any = { statusCode: 200, body: undefined, headers: {} as Record<string, string> };
23+
res.status = vi.fn((c: number) => { res.statusCode = c; return res; });
24+
res.json = vi.fn((b: any) => { res.body = b; return res; });
25+
res.header = vi.fn((k: string, v: string) => { res.headers[k] = v; return res; });
26+
res.end = vi.fn(() => res);
27+
return res;
28+
}
29+
30+
function buildServer(protocolOverrides: Record<string, any>) {
31+
const protocol: any = {
32+
getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: {} }),
33+
getMetaTypes: vi.fn().mockResolvedValue([]),
34+
// No object registered → enforceApiAccess default-allows and the handler
35+
// proceeds straight to the (mocked) write method.
36+
getMetaItems: vi.fn().mockResolvedValue([]),
37+
...protocolOverrides,
38+
};
39+
const rest = new RestServer(mockServer() as any, protocol, { api: { requireAuth: false } } as any);
40+
rest.registerRoutes();
41+
const find = (method: string, suffix: string) =>
42+
rest.getRoutes().find((r) => r.method === method && r.path.endsWith(suffix))!;
43+
return {
44+
protocol,
45+
patch: find('PATCH', '/:object/:id'),
46+
create: find('POST', '/:object'),
47+
};
48+
}
49+
50+
const DROPPED = [{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' as const }];
51+
52+
describe('PATCH /data/:object/:id — X-ObjectStack-Dropped-Fields (#3431)', () => {
53+
it('sets the header and keeps droppedFields on the body when the write stripped a field', async () => {
54+
const updateData = vi.fn().mockResolvedValue({
55+
object: 'approval_case', id: 'rec-1', record: { id: 'rec-1', title: 'B' }, droppedFields: DROPPED,
56+
});
57+
const { patch } = buildServer({ updateData });
58+
const res = mockRes();
59+
await patch.handler(
60+
{ params: { object: 'approval_case', id: 'rec-1' }, body: { title: 'B', approval_status: 'approved' }, headers: {} } as any,
61+
res,
62+
);
63+
expect(res.headers['X-ObjectStack-Dropped-Fields']).toBe('approval_status;reason=readonly');
64+
expect(res.body.droppedFields).toEqual(DROPPED);
65+
// Success semantics unchanged — no error status was set.
66+
expect(res.statusCode).toBe(200);
67+
});
68+
69+
it('joins multiple dropped fields/reasons into one header value', async () => {
70+
const updateData = vi.fn().mockResolvedValue({
71+
object: 'approval_case', id: 'rec-1', record: {},
72+
droppedFields: [
73+
{ object: 'approval_case', fields: ['locked_at', 'owner'], reason: 'readonly_when' },
74+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
75+
],
76+
});
77+
const { patch } = buildServer({ updateData });
78+
const res = mockRes();
79+
await patch.handler({ params: { object: 'approval_case', id: 'rec-1' }, body: {}, headers: {} } as any, res);
80+
expect(res.headers['X-ObjectStack-Dropped-Fields']).toBe(
81+
'locked_at;reason=readonly_when, owner;reason=readonly_when, approval_status;reason=readonly',
82+
);
83+
});
84+
85+
it('does NOT set the header when nothing was dropped', async () => {
86+
const updateData = vi.fn().mockResolvedValue({ object: 'approval_case', id: 'rec-1', record: { id: 'rec-1' } });
87+
const { patch } = buildServer({ updateData });
88+
const res = mockRes();
89+
await patch.handler({ params: { object: 'approval_case', id: 'rec-1' }, body: { title: 'B' }, headers: {} } as any, res);
90+
expect(res.headers).not.toHaveProperty('X-ObjectStack-Dropped-Fields');
91+
expect(res.body).not.toHaveProperty('droppedFields');
92+
});
93+
});
94+
95+
describe('POST /data/:object — X-ObjectStack-Dropped-Fields on create (#3431)', () => {
96+
it('sets the header (status still 201) when the create ingress stripped a readonly field', async () => {
97+
const createData = vi.fn().mockResolvedValue({
98+
object: 'approval_case', id: 'rec-1', record: { id: 'rec-1', title: 'A' }, droppedFields: DROPPED,
99+
});
100+
const { create } = buildServer({ createData });
101+
const res = mockRes();
102+
await create.handler(
103+
{ params: { object: 'approval_case' }, body: { title: 'A', approval_status: 'approved' }, headers: {} } as any,
104+
res,
105+
);
106+
expect(res.statusCode).toBe(201);
107+
expect(res.headers['X-ObjectStack-Dropped-Fields']).toBe('approval_status;reason=readonly');
108+
expect(res.body.droppedFields).toEqual(DROPPED);
109+
});
110+
});

0 commit comments

Comments
 (0)