Skip to content

Commit 426ad58

Browse files
os-litantclaude
andauthored
fix(client): declare the response the meta reset door actually sends on both deleteItem twins (#13148)
* fix(client): declare the response the meta reset door actually sends Both `deleteItem` declarations — the unscoped `ObjectStackClient.meta` and the environment-scoped `ScopedEnvironmentClient.meta` twin — declared `Promise<{ type: string; name: string; deleted: boolean }>`. That shape is not merely imprecise, it is uninhabited: `DELETE /meta/:type/:name` ends in `res.json(result)` with `deleteMetaItem`'s return, and not one of that method's four return branches carries `type`, `name` or `deleted`. So a caller who branched on the documented `deleted` flag read `undefined` — falsy — on every reset, including the ones that really removed an overlay row. Both twins now BIND `DeleteMetaItemResponse`, the type `@objectstack/spec` already exported, rather than transcribing its members: a hand-written member list is the same defect one layer up. The wire is untouched. `os meta delete` read the phantom key too and now reports `result.reset`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd * test(client): re-judge the two wire-extras pins #13208 falsified by design DeleteMetaItemResponseSchema now declares seq and projectionApplied (#13208, issue #13155), so the @ts-expect-error pins asserting they were undeclared became unused suppressions. Re-judged as positive reads: the keys are reachable from the bound type, and a schema regression dropping either reds these as TS2339. Function renamed to say what it now pins. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 43028a8 commit 426ad58

7 files changed

Lines changed: 388 additions & 13 deletions

File tree

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
"@objectstack/client": minor
3+
"@objectstack/cli": minor
4+
---
5+
6+
fix(client)!: `meta.deleteItem` declares the response the reset door actually sends (#13023)
7+
8+
**BREAKING** for a typed caller, and it breaks nothing that ever worked. Both
9+
`deleteItem` declarations — the unscoped `ObjectStackClient.meta` and the
10+
environment-scoped `ScopedEnvironmentClient.meta` twin — declared
11+
`Promise<{ type: string; name: string; deleted: boolean }>`. That shape is not
12+
merely imprecise, it is **uninhabited**: `DELETE /meta/:type/:name` ends in
13+
`res.json(result)` with `deleteMetaItem`'s return, and not one of that method's
14+
four return branches carries `type`, `name` or `deleted`. Both twins now declare
15+
`DeleteMetaItemResponse` — the type `@objectstack/spec` already exported.
16+
17+
### Migration: FROM → TO
18+
19+
```ts
20+
const r = await client.meta.deleteItem('view', 'shared_grid');
21+
22+
// FROM — compiled, and read `undefined` on EVERY reset, including the ones
23+
// that really deleted an overlay row. The branch was never taken.
24+
if (r.deleted) { invalidateCache(); }
25+
26+
// TO — the truthful flag, and it tells the two successes apart
27+
if (r.reset) { invalidateCache(); } // an overlay row was deleted
28+
else { /* none existed — already at the artifact default */ }
29+
```
30+
31+
`r.type` / `r.name` have no replacement: the door never echoed them, and the
32+
caller already holds both — it passed them in.
33+
34+
⛔ Do not write `r.reset ?? r.deleted`. There is one producer shape, and a
35+
consumer accepting two spellings is what contract-first exists to prevent. No
36+
deprecated `deleted?: boolean` transition key ships either: a transition period
37+
is for keys that *worked*, and this one never did.
38+
39+
⚠️ The real work is behavioural, not textual. Every `if (r.deleted)` has been
40+
false since it was written, so re-read what each of those branches was supposed
41+
to do — cache invalidation, registry refreshes and UI reloads guarded that way
42+
have **never run**, and moving to `r.reset` turns them on for the first time.
43+
Note also that `r.reset` and `r.success` are different questions: `success` asks
44+
whether the call was accepted, `reset` whether a row actually went away.
45+
46+
The type name is reachable without a new export from this package —
47+
`import type { DeleteMetaItemResponse } from '@objectstack/spec/api'` — which is
48+
also why no member list is transcribed here. A hand-written local copy of the
49+
schema's members is the very defect this change removes.
50+
51+
### `os meta delete`
52+
53+
The CLI read the phantom key too: its `--format json` / `--format yaml` payload
54+
carried `deleted: result.deleted`, which evaluated to `undefined`, and both
55+
`JSON.stringify` and `yaml.stringify` drop undefined values — so the `deleted`
56+
key this command has always declared **never appeared in a single run**. It now
57+
carries `result.reset`, the door's own verdict. Observable change: `os meta
58+
delete --format json` gains `deleted: true` (YAML likewise) when an overlay row
59+
was removed, and `deleted: false` when the item was already at its artifact
60+
default. The key name stays `deleted` deliberately — it is the CLI's output key,
61+
not the protocol's, and the payload's top-level `success` already means
62+
something different (the CLI envelope's "the command completed"). Same treatment
63+
`os data delete` received one door over.
64+
65+
⛔ The wire is untouched: neither `deleteMetaItem` nor
66+
`DeleteMetaItemResponseSchema` changes. Reality is the contract.
67+
68+
<!-- adr-0087: registered client-meta-reset-result-reset -->

packages/cli/src/commands/meta/delete.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,21 @@ export default class MetaDelete extends Command {
5757

5858
const result = await client.meta.deleteItem(args.type, args.name);
5959

60+
// [#13023] `deleted` is THIS COMMAND's output key; its value is the reset
61+
// door's `DeleteMetaItemResponse.reset`. Two different booleans live in
62+
// this payload and must not be conflated — the top-level `success` is the
63+
// CLI envelope's "the command completed", while `deleted` reports whether
64+
// a customization overlay row actually went away (`reset: false` means
65+
// none existed and the item was already at its artifact default). This
66+
// read was `result.deleted` until now — a key no branch of the door has
67+
// ever sent, so it evaluated to `undefined` and `JSON.stringify` /
68+
// `yaml.stringify` dropped it: the key this command has always declared
69+
// never appeared in a single run. Exactly the treatment #5638 gave the
70+
// sibling `os data delete`, one door over.
6071
if (flags.format === 'json') {
61-
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.deleted }, 'json');
72+
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.reset }, 'json');
6273
} else if (flags.format === 'yaml') {
63-
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.deleted }, 'yaml');
74+
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.reset }, 'yaml');
6475
} else {
6576
printSuccess(`Metadata deleted: ${args.type}/${args.name}`);
6677
}

packages/client/src/index.ts

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ import {
1616
GetMetaItemsResponse,
1717
GetMetaItemResponse,
1818
SaveMetaItemResponse,
19+
// [#13023] The reset door's response contract. Both `meta.deleteItem`
20+
// declarations BIND this type rather than transcribing its members: a
21+
// hand-written member list is the exact defect this card removes (a local
22+
// declaration that drifts from the wire), and the card's own body
23+
// demonstrated the failure by attributing the IMPLEMENTATION's declared
24+
// return to this schema.
25+
DeleteMetaItemResponse,
1926
PublishMetaItemResponse,
2027
PublishPackageDraftsResponse,
2128
LoginRequest,
@@ -1150,12 +1157,24 @@ export class ObjectStackClient {
11501157
* metadata_conflict` instead — the door has always read the header
11511158
* (`DeleteMetaItemRequest.parentVersion` describes it), this client just
11521159
* had no argument for it until #12181.
1160+
*
1161+
* [#13023] READ `reset`, NEVER `deleted`. This method used to declare
1162+
* `{ type, name, deleted }` — an UNINHABITED shape: the door answers
1163+
* `res.json(result)` with `deleteMetaItem`'s return, and not one of its
1164+
* four branches carries `type`, `name` or `deleted`. So `r.deleted`
1165+
* compiled and read `undefined` on EVERY reset, including the ones that
1166+
* really removed a row, and the SDK's own tests had to cast through `any`
1167+
* to see the truth. The truthful flag is {@link DeleteMetaItemResponse}'s
1168+
* `reset`: `true` means an overlay row was deleted, `false` means none
1169+
* existed and the item was already at its artifact default — exactly the
1170+
* distinction a caller most wants. Same correction #5638 made one door
1171+
* over on `DeleteDataResult`.
11531172
*/
11541173
deleteItem: async (
11551174
type: string,
11561175
name: string,
11571176
options?: DeleteMetaItemOptions,
1158-
): Promise<{ type: string; name: string; deleted: boolean }> => {
1177+
): Promise<DeleteMetaItemResponse> => {
11591178
const route = this.getRoute('metadata');
11601179
// `query`, not `qs` — it carries its own `?`; see `saveItem`'s note on
11611180
// the three meanings `qs` holds in this file.
@@ -1169,7 +1188,11 @@ export class ObjectStackClient {
11691188
method: 'DELETE',
11701189
...(headers ? { headers } : {}),
11711190
});
1172-
return this.unwrapResponse(res);
1191+
// The door answers BARE (`res.json(result)`), and `unwrapResponse`
1192+
// strips only a body carrying BOTH a boolean `success` AND a `data`
1193+
// key — this one has no `data` — so the caller receives the door's
1194+
// whole body and the annotation above describes it.
1195+
return this.unwrapResponse<DeleteMetaItemResponse>(res);
11731196
},
11741197

11751198
/**
@@ -6013,12 +6036,18 @@ export class ScopedEnvironmentClient {
60136036
* reads `?state=` — and the `If-Match` header — byte-identically. A bag
60146037
* on only one of the two clients would be a fresh divergence of the kind
60156038
* #7019 rules against, not half a fix.
6039+
*
6040+
* [#13023] Returns {@link DeleteMetaItemResponse} — read `reset`, never
6041+
* `deleted`. The phantom `{ type, name, deleted }` declaration was
6042+
* TEXTUALLY IDENTICAL on both twins, so correcting one and not the other
6043+
* would have been half a fix in the same #11713 direction the bag above
6044+
* records. See the unscoped twin for the full account.
60166045
*/
60176046
deleteItem: async (
60186047
type: string,
60196048
name: string,
60206049
options?: DeleteMetaItemOptions,
6021-
): Promise<{ type: string; name: string; deleted: boolean }> => {
6050+
): Promise<DeleteMetaItemResponse> => {
60226051
// `query`, not `qs` — it carries its own `?`; see the unscoped twin.
60236052
const query = metaDeleteQuery(options);
60246053
// Header half of the same bag, through the same one builder the twin
@@ -6028,7 +6057,8 @@ export class ScopedEnvironmentClient {
60286057
method: 'DELETE',
60296058
...(headers ? { headers } : {}),
60306059
});
6031-
return this.parent._unwrap(res);
6060+
// Bare body, same as the unscoped twin — `_unwrap` is `unwrapResponse`.
6061+
return this.parent._unwrap<DeleteMetaItemResponse>(res);
60326062
},
60336063
getHistory: async (
60346064
type: string,

packages/client/src/meta-delete-item-carriers.test.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ import {
7373
} from '@objectstack/metadata-core';
7474
import { RestServer } from '@objectstack/runtime';
7575
import { ObjectStackClient } from './index';
76+
// [#13023] The reset door's response contract. Every `deleteItem` result below
77+
// is bound to it instead of `any`: these reads used to be `const r: any`
78+
// PRECISELY because the declared return (`{ type, name, deleted }`) named none
79+
// of the fields the door actually sends, so reading the truth required dodging
80+
// the type. With the declaration corrected the cast is not merely unnecessary,
81+
// it would hide the fix — and `reset`, the flag this file already asserts in
82+
// BOTH directions against the real door, is now a typed read.
83+
import type { DeleteMetaItemResponse } from '@objectstack/spec/api';
7684

7785
// ---------------------------------------------------------------------------
7886
// Part 1 — what the CLIENT puts on the wire (both declarations)
@@ -467,11 +475,28 @@ describe('[#12181] the real reset door: a concurrent edit is destroyed unpinned,
467475
// A resets, holding a version that is no longer current. This is the
468476
// BEFORE state of the card: with no options bag there was no other
469477
// call to make.
470-
const reset: any = await client.meta.deleteItem('view', 'race_probe');
478+
const reset: DeleteMetaItemResponse = await client.meta.deleteItem('view', 'race_probe');
471479

472480
// Silently destroyed: success, and B's edit is gone from the store.
481+
// These two are TYPED reads since #13023 — under the phantom
482+
// `{ type, name, deleted }` declaration they were TS2339 and this
483+
// binding had to be `any` to compile at all.
473484
expect(reset.success).toBe(true);
474485
expect(reset.reset).toBe(true);
486+
487+
// [#13023] The phantom shape, refuted on the REAL door rather than
488+
// argued from the schema. `deleted` — the flag the declaration told
489+
// every caller to branch on — is not a key on this body, and neither
490+
// are `type` and `name`. A first-party consumer writing
491+
// `if (r.deleted)` took the FALSE branch here, on the reset that
492+
// really did destroy a row.
493+
expect('deleted' in (reset as object)).toBe(false);
494+
expect('type' in (reset as object)).toBe(false);
495+
expect('name' in (reset as object)).toBe(false);
496+
// The positive control that keeps those three absences honest: the
497+
// same instrument, same body, sees the keys that ARE there.
498+
expect('success' in (reset as object)).toBe(true);
499+
expect('reset' in (reset as object)).toBe(true);
475500
expect(await overlayRows(engine, 'race_probe')).toHaveLength(0);
476501
// The probe: no pin ever reached the protocol.
477502
expect(deleteRequests).toHaveLength(1);
@@ -514,7 +539,7 @@ describe('[#12181] the real reset door: a concurrent edit is destroyed unpinned,
514539
// write. Without this, "always 409" would pass the case above.
515540
const { engine, client } = await bootDoor();
516541
const saved: any = await client.meta.saveItem('view', 'fresh_probe', VIEW('fresh_probe', 'A'));
517-
const reset: any = await client.meta.deleteItem('view', 'fresh_probe', { ifMatch: saved.version });
542+
const reset: DeleteMetaItemResponse = await client.meta.deleteItem('view', 'fresh_probe', { ifMatch: saved.version });
518543
expect(reset.success).toBe(true);
519544
expect(await overlayRows(engine, 'fresh_probe')).toHaveLength(0);
520545
}, 60_000);
@@ -542,7 +567,7 @@ describe('[#12181] the real reset door: a concurrent edit is destroyed unpinned,
542567

543568
// …and unpinned, the scoped twin destroys it exactly like the
544569
// unscoped one — same handler, same last-write-wins default.
545-
const reset: any = await scoped.deleteItem('view', 'scoped_race');
570+
const reset: DeleteMetaItemResponse = await scoped.deleteItem('view', 'scoped_race');
546571
expect(reset.success).toBe(true);
547572
expect(await overlayRows(engine, 'scoped_race')).toHaveLength(0);
548573
}, 60_000);
@@ -561,7 +586,7 @@ describe('[#12181] the real reset door: `?state=draft` discards ONLY the pending
561586
expect(before.map((r: any) => r.state).sort()).toEqual(['active', 'draft']);
562587

563588
// The narrow reset — unreachable from this SDK before this card.
564-
const discarded: any = await client.meta.deleteItem('view', 'draft_probe', { state: 'draft' });
589+
const discarded: DeleteMetaItemResponse = await client.meta.deleteItem('view', 'draft_probe', { state: 'draft' });
565590
expect(discarded.success).toBe(true);
566591
// The door parsed `?state=draft` and threaded it into the protocol
567592
// call. (Positive control for the sibling case below, where the same
@@ -576,14 +601,14 @@ describe('[#12181] the real reset door: `?state=draft` discards ONLY the pending
576601

577602
// A second draft discard has nothing left to discard — the door says
578603
// so rather than falling through to the active row.
579-
const again: any = await client.meta.deleteItem('view', 'draft_probe', { state: 'draft' });
604+
const again: DeleteMetaItemResponse = await client.meta.deleteItem('view', 'draft_probe', { state: 'draft' });
580605
expect(again.reset).toBe(false);
581606
expect(await overlayRows(engine, 'draft_probe')).toHaveLength(1);
582607

583608
// …and the FULL reset — the only one the SDK could express before —
584609
// takes the published overlay with it. This is why withholding
585610
// `?state=draft` did not make the client safer.
586-
const full: any = await client.meta.deleteItem('view', 'draft_probe');
611+
const full: DeleteMetaItemResponse = await client.meta.deleteItem('view', 'draft_probe');
587612
expect(full.reset).toBe(true);
588613
expect(await overlayRows(engine, 'draft_probe')).toHaveLength(0);
589614
// The probe again: `state` is absent on the full reset — measured on
@@ -598,7 +623,7 @@ describe('[#12181] the real reset door: `?state=draft` discards ONLY the pending
598623
await scoped.saveItem('view', 'scoped_draft', VIEW('scoped_draft', 'published'));
599624
await scoped.saveItem('view', 'scoped_draft', VIEW('scoped_draft', 'pending'), { mode: 'draft' });
600625

601-
const discarded: any = await scoped.deleteItem('view', 'scoped_draft', { state: 'draft' });
626+
const discarded: DeleteMetaItemResponse = await scoped.deleteItem('view', 'scoped_draft', { state: 'draft' });
602627
expect(discarded.success).toBe(true);
603628
expect(deleteRequests[0].state).toBe('draft');
604629
const after = await overlayRows(engine, 'scoped_draft');

0 commit comments

Comments
 (0)