Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions .changeset/meta-delete-item-return-type.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
---
"@objectstack/client": minor
"@objectstack/cli": minor
---

fix(client)!: `meta.deleteItem` declares the response the reset door actually sends (#13023)

**BREAKING** for a typed caller, and it breaks nothing that ever worked. 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`. Both twins now declare
`DeleteMetaItemResponse` — the type `@objectstack/spec` already exported.

### Migration: FROM → TO

```ts
const r = await client.meta.deleteItem('view', 'shared_grid');

// FROM — compiled, and read `undefined` on EVERY reset, including the ones
// that really deleted an overlay row. The branch was never taken.
if (r.deleted) { invalidateCache(); }

// TO — the truthful flag, and it tells the two successes apart
if (r.reset) { invalidateCache(); } // an overlay row was deleted
else { /* none existed — already at the artifact default */ }
```

`r.type` / `r.name` have no replacement: the door never echoed them, and the
caller already holds both — it passed them in.

⛔ Do not write `r.reset ?? r.deleted`. There is one producer shape, and a
consumer accepting two spellings is what contract-first exists to prevent. No
deprecated `deleted?: boolean` transition key ships either: a transition period
is for keys that *worked*, and this one never did.

⚠️ The real work is behavioural, not textual. Every `if (r.deleted)` has been
false since it was written, so re-read what each of those branches was supposed
to do — cache invalidation, registry refreshes and UI reloads guarded that way
have **never run**, and moving to `r.reset` turns them on for the first time.
Note also that `r.reset` and `r.success` are different questions: `success` asks
whether the call was accepted, `reset` whether a row actually went away.

The type name is reachable without a new export from this package —
`import type { DeleteMetaItemResponse } from '@objectstack/spec/api'` — which is
also why no member list is transcribed here. A hand-written local copy of the
schema's members is the very defect this change removes.

### `os meta delete`

The CLI read the phantom key too: its `--format json` / `--format yaml` payload
carried `deleted: result.deleted`, which evaluated to `undefined`, and both
`JSON.stringify` and `yaml.stringify` drop undefined values — so the `deleted`
key this command has always declared **never appeared in a single run**. It now
carries `result.reset`, the door's own verdict. Observable change: `os meta
delete --format json` gains `deleted: true` (YAML likewise) when an overlay row
was removed, and `deleted: false` when the item was already at its artifact
default. The key name stays `deleted` deliberately — it is the CLI's output key,
not the protocol's, and the payload's top-level `success` already means
something different (the CLI envelope's "the command completed"). Same treatment
`os data delete` received one door over.

⛔ The wire is untouched: neither `deleteMetaItem` nor
`DeleteMetaItemResponseSchema` changes. Reality is the contract.

<!-- adr-0087: registered client-meta-reset-result-reset -->
15 changes: 13 additions & 2 deletions packages/cli/src/commands/meta/delete.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,10 +57,21 @@ export default class MetaDelete extends Command {

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

// [#13023] `deleted` is THIS COMMAND's output key; its value is the reset
// door's `DeleteMetaItemResponse.reset`. Two different booleans live in
// this payload and must not be conflated — the top-level `success` is the
// CLI envelope's "the command completed", while `deleted` reports whether
// a customization overlay row actually went away (`reset: false` means
// none existed and the item was already at its artifact default). This
// read was `result.deleted` until now — a key no branch of the door has
// ever sent, so it evaluated to `undefined` and `JSON.stringify` /
// `yaml.stringify` dropped it: the key this command has always declared
// never appeared in a single run. Exactly the treatment #5638 gave the
// sibling `os data delete`, one door over.
if (flags.format === 'json') {
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.deleted }, 'json');
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.reset }, 'json');
} else if (flags.format === 'yaml') {
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.deleted }, 'yaml');
await formatOutput({ success: true, type: args.type, name: args.name, deleted: result.reset }, 'yaml');
} else {
printSuccess(`Metadata deleted: ${args.type}/${args.name}`);
}
Expand Down
38 changes: 34 additions & 4 deletions packages/client/src/index.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,13 @@ import {
GetMetaItemsResponse,
GetMetaItemResponse,
SaveMetaItemResponse,
// [#13023] The reset door's response contract. Both `meta.deleteItem`
// declarations BIND this type rather than transcribing its members: a
// hand-written member list is the exact defect this card removes (a local
// declaration that drifts from the wire), and the card's own body
// demonstrated the failure by attributing the IMPLEMENTATION's declared
// return to this schema.
DeleteMetaItemResponse,
PublishMetaItemResponse,
PublishPackageDraftsResponse,
LoginRequest,
Expand DownExpand Up@@ -1150,12 +1157,24 @@ export class ObjectStackClient {
* metadata_conflict` instead — the door has always read the header
* (`DeleteMetaItemRequest.parentVersion` describes it), this client just
* had no argument for it until #12181.
*
* [#13023] READ `reset`, NEVER `deleted`. This method used to declare
* `{ type, name, deleted }` — an UNINHABITED shape: the door answers
* `res.json(result)` with `deleteMetaItem`'s return, and not one of its
* four branches carries `type`, `name` or `deleted`. So `r.deleted`
* compiled and read `undefined` on EVERY reset, including the ones that
* really removed a row, and the SDK's own tests had to cast through `any`
* to see the truth. The truthful flag is {@link DeleteMetaItemResponse}'s
* `reset`: `true` means an overlay row was deleted, `false` means none
* existed and the item was already at its artifact default — exactly the
* distinction a caller most wants. Same correction #5638 made one door
* over on `DeleteDataResult`.
*/
deleteItem: async (
type: string,
name: string,
options?: DeleteMetaItemOptions,
): Promise<{ type: string; name: string; deleted: boolean }> => {
): Promise<DeleteMetaItemResponse> => {
const route = this.getRoute('metadata');
// `query`, not `qs` — it carries its own `?`; see `saveItem`'s note on
// the three meanings `qs` holds in this file.
Expand All@@ -1169,7 +1188,11 @@ export class ObjectStackClient {
method: 'DELETE',
...(headers ? { headers } : {}),
});
return this.unwrapResponse(res);
// The door answers BARE (`res.json(result)`), and `unwrapResponse`
// strips only a body carrying BOTH a boolean `success` AND a `data`
// key — this one has no `data` — so the caller receives the door's
// whole body and the annotation above describes it.
return this.unwrapResponse<DeleteMetaItemResponse>(res);
},

/**
Expand DownExpand Up@@ -6013,12 +6036,18 @@ export class ScopedEnvironmentClient {
* reads `?state=` — and the `If-Match` header — byte-identically. A bag
* on only one of the two clients would be a fresh divergence of the kind
* #7019 rules against, not half a fix.
*
* [#13023] Returns {@link DeleteMetaItemResponse} — read `reset`, never
* `deleted`. The phantom `{ type, name, deleted }` declaration was
* TEXTUALLY IDENTICAL on both twins, so correcting one and not the other
* would have been half a fix in the same #11713 direction the bag above
* records. See the unscoped twin for the full account.
*/
deleteItem: async (
type: string,
name: string,
options?: DeleteMetaItemOptions,
): Promise<{ type: string; name: string; deleted: boolean }> => {
): Promise<DeleteMetaItemResponse> => {
// `query`, not `qs` — it carries its own `?`; see the unscoped twin.
const query = metaDeleteQuery(options);
// Header half of the same bag, through the same one builder the twin
Expand All@@ -6028,7 +6057,8 @@ export class ScopedEnvironmentClient {
method: 'DELETE',
...(headers ? { headers } : {}),
});
return this.parent._unwrap(res);
// Bare body, same as the unscoped twin — `_unwrap` is `unwrapResponse`.
return this.parent._unwrap<DeleteMetaItemResponse>(res);
},
getHistory: async (
type: string,
Expand Down
39 changes: 32 additions & 7 deletions packages/client/src/meta-delete-item-carriers.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,6 +73,14 @@ import {
} from '@objectstack/metadata-core';
import { RestServer } from '@objectstack/runtime';
import { ObjectStackClient } from './index';
// [#13023] The reset door's response contract. Every `deleteItem` result below
// is bound to it instead of `any`: these reads used to be `const r: any`
// PRECISELY because the declared return (`{ type, name, deleted }`) named none
// of the fields the door actually sends, so reading the truth required dodging
// the type. With the declaration corrected the cast is not merely unnecessary,
// it would hide the fix — and `reset`, the flag this file already asserts in
// BOTH directions against the real door, is now a typed read.
import type { DeleteMetaItemResponse } from '@objectstack/spec/api';

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

// Silently destroyed: success, and B's edit is gone from the store.
// These two are TYPED reads since #13023 — under the phantom
// `{ type, name, deleted }` declaration they were TS2339 and this
// binding had to be `any` to compile at all.
expect(reset.success).toBe(true);
expect(reset.reset).toBe(true);

// [#13023] The phantom shape, refuted on the REAL door rather than
// argued from the schema. `deleted` — the flag the declaration told
// every caller to branch on — is not a key on this body, and neither
// are `type` and `name`. A first-party consumer writing
// `if (r.deleted)` took the FALSE branch here, on the reset that
// really did destroy a row.
expect('deleted' in (reset as object)).toBe(false);
expect('type' in (reset as object)).toBe(false);
expect('name' in (reset as object)).toBe(false);
// The positive control that keeps those three absences honest: the
// same instrument, same body, sees the keys that ARE there.
expect('success' in (reset as object)).toBe(true);
expect('reset' in (reset as object)).toBe(true);
expect(await overlayRows(engine, 'race_probe')).toHaveLength(0);
// The probe: no pin ever reached the protocol.
expect(deleteRequests).toHaveLength(1);
Expand DownExpand Up@@ -514,7 +539,7 @@ describe('[#12181] the real reset door: a concurrent edit is destroyed unpinned,
// write. Without this, "always 409" would pass the case above.
const { engine, client } = await bootDoor();
const saved: any = await client.meta.saveItem('view', 'fresh_probe', VIEW('fresh_probe', 'A'));
const reset: any = await client.meta.deleteItem('view', 'fresh_probe', { ifMatch: saved.version });
const reset: DeleteMetaItemResponse = await client.meta.deleteItem('view', 'fresh_probe', { ifMatch: saved.version });
expect(reset.success).toBe(true);
expect(await overlayRows(engine, 'fresh_probe')).toHaveLength(0);
}, 60_000);
Expand DownExpand Up@@ -542,7 +567,7 @@ describe('[#12181] the real reset door: a concurrent edit is destroyed unpinned,

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

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

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

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

const discarded: any = await scoped.deleteItem('view', 'scoped_draft', { state: 'draft' });
const discarded: DeleteMetaItemResponse = await scoped.deleteItem('view', 'scoped_draft', { state: 'draft' });
expect(discarded.success).toBe(true);
expect(deleteRequests[0].state).toBe('draft');
const after = await overlayRows(engine, 'scoped_draft');
Expand Down
Loading
Loading