Skip to content

Commit 887b97d

Browse files
claude[bot]claude
claude[bot]
and
claude
authored
feat(client): let meta.saveItem send the query string its route already reads (#11714)
* feat(client): let meta.saveItem send the query string its route already reads The Phase 3a-destructive gate refuses with `409 DESTRUCTIVE_CHANGE` and ends `— re-submit with ?force=true to proceed.` Both REST `PUT` doors read `?force` and thread it, so the clause is true of an HTTP caller. It was false of a first-party SDK caller: both `saveItem` declarations built a bare path and a body and sent no query string at all, so doing exactly what the refusal said returned the identical refusal and the only remedy was raw `fetch`. Adds an optional `SaveMetaItemOptions` bag — `force`, `packageId`, `mode` — matching `getItem`'s house shape on the same object and covering exactly the three parameters `PUT /api/v1/meta/:type/:name` reads. One exported type and one query builder are shared by the unscoped client and the environment-scoped twin so the two cannot drift; an options-less call builds a byte-identical URL to before. Fixes #11391 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR * docs(client): warn that ?mode=draft never reaches the compound-name door Review follow-up. This PR exposes `mode` on a client that addresses BOTH `PUT` doors, and `PUT /meta/:type/:section/:name` never reads the parameter while its single-segment twin does — so a compound-name save with `{ mode: 'draft' }` is ignored and PUBLISHED LIVE, answered 200, with no signal at the call site. That is this card's own defect shape one parameter over, and it became reachable only because this PR made `mode` settable. Measured over the compound handler's body (rest-server.ts 6646-6824): `force` and `package` ARE both read and threaded there; `mode` is the only one of the three that is not (zero hits, reverse-checked with `compoundName`). The docstring therefore names the gap narrowly and states why the remedy is NOT to refuse the whole bag on a compound name — that would break the two parameters that work in order to warn about the one that does not. Threading it is the route's decision, filed as #11712 rather than guessed at here. Also renames the two new locals from `qs` to `query`. Measured on this file: of 37 `const qs =` bindings, 27 hold a bare `params.toString()`, 8 hold a string already carrying its `?`, and 2 hold a `URLSearchParams` object — one name, three meanings. These carry their `?`, so they say so. Comments and local names only; no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f648cbe commit 887b97d

3 files changed

Lines changed: 408 additions & 5 deletions

File tree

‎.changeset/client-meta-saveitem-query-options.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
'@objectstack/client': minor
3+
---
4+
5+
`meta.saveItem` accepts the query-string options bag its route already reads —
6+
`force`, `packageId`, `mode` — on both clients
7+
8+
The Phase 3a-destructive gate refuses a metadata save with
9+
`409 DESTRUCTIVE_CHANGE` and ends the message `— re-submit with ?force=true to
10+
proceed.` Both REST `PUT` doors read `?force` off the query string and thread
11+
it, so that sentence is true of an HTTP caller. It was **false of a
12+
first-party SDK caller**: `meta.saveItem(type, name, item)` built a bare path
13+
and a body and sent no query string at all, on either declaration. A caller
14+
who did literally what the refusal prescribed got the identical refusal back,
15+
and the only way to act on it was to abandon `@objectstack/client` for raw
16+
`fetch`.
17+
18+
Three parameters are newly reachable, and they are exactly the three
19+
`PUT /api/v1/meta/:type/:name` reads:
20+
21+
- **`force?: boolean`** — `?force=true`, the destructive-change opt-in the 409
22+
message names. Only the opt-IN is spelled on the wire: `false` and
23+
`undefined` both omit the parameter rather than sending `?force=false`.
24+
That is a hazard avoided, not tidiness — the door refuses a *repeated*
25+
`force` because a repeated value arrives as an array and a non-empty array
26+
is truthy, so an opt-OUT that reached the wire twice would switch the guard
27+
ON.
28+
- **`packageId?: string`** — `?package=<id>`, binding the saved row to a
29+
software package (`sys_metadata.package_id`). Named `packageId` to match the
30+
sibling `getItem` / `getItems` options on the same object.
31+
- **`mode?: 'draft' | 'publish'`** — `?mode=draft`, staging the write as a
32+
pending draft. `'publish'` is the default said out loud and deliberately
33+
sends nothing, since the door acts on `mode=draft` alone.
34+
35+
**Backward compatible.** The bag is optional and an options-less call builds a
36+
byte-identical URL to before — `''`, not a trailing `?`. Existing
37+
three-argument callers are unaffected, and pins measure that rather than
38+
assuming it.
39+
40+
Both declarations move together — the unscoped `ObjectStackClient.meta` and
41+
the environment-scoped `ScopedProjectClient.meta` — sharing ONE exported
42+
`SaveMetaItemOptions` type and ONE query builder rather than a literal copied
43+
into each. They are the same method on two clients reaching one pair of routes
44+
(the scoped mount is the same route registration replayed under
45+
`/environments/:environmentId`, so it reads the same three parameters), and
46+
every divergence measured between these twins so far has been closed as a
47+
defect. A bag spelled twice is the next one waiting to be introduced.
48+
49+
The branch was selected by measurement, not preference: the SDK is the real
50+
metadata-write path for both surfaces the ruling named. The CLI's `os meta
51+
register` goes through `client.meta.saveItem` and the CLI has no raw-HTTP
52+
metadata-save path at all; Studio reaches it from 21 production call sites
53+
across `@object-ui/app-shell`, `plugin-designer`, `data-objectstack` and the
54+
console app — including the object and field designers, where dropping a field
55+
and saving is precisely what raises the destructive 409.

‎packages/client/src/client.test.ts‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2562,3 +2562,200 @@ describe('Analytics namespace (#3584 dispatcher alignment)', () => {
25622562
);
25632563
});
25642564
});
2565+
2566+
// ----------------------------------------------------------------------
2567+
// [#11391] `meta.saveItem`'s query string — the destructive-409 remedy has
2568+
// to be reachable from the SDK, on BOTH clients.
2569+
//
2570+
// The Phase 3a-destructive gate refuses with `409 DESTRUCTIVE_CHANGE` and
2571+
// ends `— re-submit with ?force=true to proceed.` Both REST `PUT` doors read
2572+
// `?force` and thread it, so that sentence is true of an HTTP caller. It was
2573+
// FALSE of a first-party SDK caller: `saveItem` built a bare path and a body
2574+
// and sent no query string at all, so doing exactly what the refusal said
2575+
// returned the identical refusal and the only way out was raw `fetch`.
2576+
//
2577+
// These pins are on the URL the client BUILDS, deliberately. A test that only
2578+
// checks the method accepts an option would stay green against a client that
2579+
// swallows it — which is the same defect one layer in.
2580+
// ----------------------------------------------------------------------
2581+
2582+
describe('[#11391] meta.saveItem query string (unscoped client)', () => {
2583+
it('threads `force: true` onto the URL as ?force=true', async () => {
2584+
const { client, fetchMock } = createMockClient({ success: true, version: 2 });
2585+
await client.meta.saveItem('object', 'customer', { name: 'customer' }, { force: true });
2586+
const [url, init] = fetchMock.mock.calls[0];
2587+
expect(String(url)).toBe('http://localhost:3000/api/v1/meta/object/customer?force=true');
2588+
expect(init.method).toBe('PUT');
2589+
// The body is untouched by the option — `force` is a query parameter,
2590+
// not a field the server reads off the document being saved.
2591+
expect(JSON.parse(init.body)).toEqual({ name: 'customer' });
2592+
});
2593+
2594+
it('exposes ?package and ?mode=draft in the same bag, in a stable order', async () => {
2595+
const { client, fetchMock } = createMockClient({ success: true });
2596+
await client.meta.saveItem('object', 'customer', { name: 'customer' }, {
2597+
force: true,
2598+
packageId: 'app.crm',
2599+
mode: 'draft',
2600+
});
2601+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2602+
'http://localhost:3000/api/v1/meta/object/customer?force=true&package=app.crm&mode=draft',
2603+
);
2604+
});
2605+
2606+
it('sends `package` alone when that is all the caller set', async () => {
2607+
const { client, fetchMock } = createMockClient({ success: true });
2608+
await client.meta.saveItem('object', 'customer', {}, { packageId: 'app.crm' });
2609+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2610+
'http://localhost:3000/api/v1/meta/object/customer?package=app.crm',
2611+
);
2612+
});
2613+
2614+
it('url-encodes a packageId that needs it', async () => {
2615+
const { client, fetchMock } = createMockClient({ success: true });
2616+
await client.meta.saveItem('object', 'customer', {}, { packageId: 'acme/crm suite' });
2617+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2618+
'http://localhost:3000/api/v1/meta/object/customer?package=acme%2Fcrm+suite',
2619+
);
2620+
});
2621+
2622+
it('BACKWARD COMPATIBLE: a 3-argument call still sends no query string at all', async () => {
2623+
const { client, fetchMock } = createMockClient({ success: true });
2624+
await client.meta.saveItem('object', 'customer', { name: 'customer' });
2625+
// Byte-identical to the pre-#11391 URL — not `…/customer?`.
2626+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2627+
'http://localhost:3000/api/v1/meta/object/customer',
2628+
);
2629+
});
2630+
2631+
it('an empty bag is also byte-identical to no bag', async () => {
2632+
const { client, fetchMock } = createMockClient({ success: true });
2633+
await client.meta.saveItem('object', 'customer', {}, {});
2634+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2635+
'http://localhost:3000/api/v1/meta/object/customer',
2636+
);
2637+
});
2638+
2639+
it('NEVER spells the opt-OUT on the wire (#6877): force:false sends nothing', async () => {
2640+
const { client, fetchMock } = createMockClient({ success: true });
2641+
await client.meta.saveItem('object', 'customer', {}, { force: false });
2642+
// `?force=false` would be a live hazard rather than a no-op: the door
2643+
// refuses a REPEATED `force` because a repeated value arrives as an
2644+
// array and a non-empty array is truthy — a spelled-out opt-OUT that
2645+
// reached the wire twice would turn the destructive guard ON. Emitting
2646+
// nothing keeps this client clear of that edge.
2647+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2648+
'http://localhost:3000/api/v1/meta/object/customer',
2649+
);
2650+
});
2651+
2652+
it("mode:'publish' is the default said out loud and sends nothing", async () => {
2653+
const { client, fetchMock } = createMockClient({ success: true });
2654+
await client.meta.saveItem('object', 'customer', {}, { mode: 'publish' });
2655+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2656+
'http://localhost:3000/api/v1/meta/object/customer',
2657+
);
2658+
});
2659+
2660+
it('a compound name keeps its unencoded slash AND gets the query string', async () => {
2661+
const { client, fetchMock } = createMockClient({ success: true });
2662+
await client.meta.saveItem('object', 'views/all_leads', { label: 'All leads' }, { force: true });
2663+
// The slash must still survive (%2F would collapse this onto the
2664+
// 3-segment route and miss `PUT /meta/:type/:section/:name`), and the
2665+
// compound door reads `?force` too since #11095.
2666+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2667+
'http://localhost:3000/api/v1/meta/object/views/all_leads?force=true',
2668+
);
2669+
});
2670+
});
2671+
2672+
describe('[#11391] meta.saveItem query string (environment-scoped twin)', () => {
2673+
it('threads `force: true` on the scoped client too', async () => {
2674+
const { client, fetchMock } = createMockClient({ success: true });
2675+
await client.project('proj-123').meta.saveItem(
2676+
'object', 'customer', { name: 'customer' }, { force: true },
2677+
);
2678+
const [url, init] = fetchMock.mock.calls[0];
2679+
expect(String(url)).toBe(
2680+
'http://localhost:3000/api/v1/environments/proj-123/meta/object/customer?force=true',
2681+
);
2682+
expect(init.method).toBe('PUT');
2683+
});
2684+
2685+
it('exposes the same three parameters as the unscoped twin', async () => {
2686+
const { client, fetchMock } = createMockClient({ success: true });
2687+
await client.project('proj-123').meta.saveItem('object', 'customer', {}, {
2688+
force: true,
2689+
packageId: 'app.crm',
2690+
mode: 'draft',
2691+
});
2692+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2693+
'http://localhost:3000/api/v1/environments/proj-123/meta/object/customer'
2694+
+ '?force=true&package=app.crm&mode=draft',
2695+
);
2696+
});
2697+
2698+
it('BACKWARD COMPATIBLE: a 3-argument scoped call still sends no query string', async () => {
2699+
const { client, fetchMock } = createMockClient({ success: true });
2700+
await client.project('proj-123').meta.saveItem('object', 'customer', {});
2701+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2702+
'http://localhost:3000/api/v1/environments/proj-123/meta/object/customer',
2703+
);
2704+
});
2705+
2706+
it('IN STEP with the unscoped twin: identical query for identical options', async () => {
2707+
// The twins are one method on two clients reaching one pair of routes
2708+
// (the scoped mount is the same `registerForBase` replayed under
2709+
// `/environments/:id`). This compares the QUERY they build rather than
2710+
// restating both URLs, so it keeps holding if either path changes.
2711+
const { client, fetchMock } = createMockClient({ success: true });
2712+
const opts = { force: true, packageId: 'app.crm', mode: 'draft' } as const;
2713+
await client.meta.saveItem('object', 'customer', {}, opts);
2714+
await client.project('proj-123').meta.saveItem('object', 'customer', {}, opts);
2715+
const queryOf = (u: unknown) => new URL(String(u)).search;
2716+
expect(queryOf(fetchMock.mock.calls[1][0])).toBe(queryOf(fetchMock.mock.calls[0][0]));
2717+
expect(queryOf(fetchMock.mock.calls[0][0])).toBe('?force=true&package=app.crm&mode=draft');
2718+
});
2719+
});
2720+
2721+
describe('[#11391] the destructive-409 remedy loop is now closed for an SDK caller', () => {
2722+
it('refused with DESTRUCTIVE_CHANGE, the caller can do what the message says', async () => {
2723+
// Round 1 answers the real refusal envelope; round 2 answers a save.
2724+
const refusal = {
2725+
error: "[destructive_change] object/customer would drop or transform existing data:"
2726+
+ " field 'legacy_code' removed — re-submit with ?force=true to proceed.",
2727+
code: 'DESTRUCTIVE_CHANGE',
2728+
};
2729+
const responses = [
2730+
{ ok: false, status: 409, statusText: 'Conflict', json: async () => refusal, headers: new Headers() },
2731+
{ ok: true, status: 200, statusText: 'OK', json: async () => ({ success: true, version: 3 }), headers: new Headers() },
2732+
];
2733+
const fetchMock = vi.fn().mockImplementation(() => Promise.resolve(responses.shift()));
2734+
const client = new ObjectStackClient({ baseUrl: 'http://localhost:3000', fetch: fetchMock });
2735+
2736+
// 1. Refused. Assert the ENVELOPE the caller branches on, not merely
2737+
// that something threw: a bare `.toThrow()` would stay green
2738+
// against any error at all.
2739+
const err: any = await client.meta.saveItem('object', 'customer', { name: 'customer' })
2740+
.then(() => { throw new Error('expected the destructive save to be refused'); }, (e) => e);
2741+
expect(err.code).toBe('DESTRUCTIVE_CHANGE');
2742+
// The SDK parks the numeric on `httpStatus`; `status` is only set on
2743+
// the auth-login path, so this is the carrier to read here.
2744+
expect(err.httpStatus).toBe(409);
2745+
expect(String(err.message)).toContain('re-submit with ?force=true to proceed.');
2746+
expect(String(fetchMock.mock.calls[0][0])).toBe(
2747+
'http://localhost:3000/api/v1/meta/object/customer',
2748+
);
2749+
2750+
// 2. Do literally what the refusal prescribed — and the parameter it
2751+
// names actually reaches the route. THIS is the acceptance
2752+
// criterion: before #11391 there was no argument to pass here.
2753+
const saved = await client.meta.saveItem(
2754+
'object', 'customer', { name: 'customer' }, { force: true },
2755+
);
2756+
expect(String(fetchMock.mock.calls[1][0])).toBe(
2757+
'http://localhost:3000/api/v1/meta/object/customer?force=true',
2758+
);
2759+
expect(saved).toEqual({ success: true, version: 3 });
2760+
});
2761+
});

0 commit comments

Comments
 (0)