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
52 changes: 52 additions & 0 deletions .changeset/18402-meta-item-one-absence-envelope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
"@objectstack/rest": minor
---

fix(rest)!: `GET /meta/:type/:name` answers absence in ONE envelope, whichever arm produced it (#18402)

<!-- adr-0087: not-required (no-migration-prescription) nothing an author writes is retired, renamed or given a new meaning here: no metadata key, no spec schema, no `packages/spec` file is in this diff, and no stored `sys_metadata` document changes shape or content. The only thing that moves is the WIRE BODY of one REST refusal — the absence answer of a single read route — which is not an ADR-0087 surface at all, so `objectstack migrate meta` has nothing it could rewrite and there is no registry entry for this to be missing. Judged against this diff's own facts: the six changed files are `packages/rest/src/{rest-server,error-response}.ts`, three `packages/rest` pin tests and one `packages/qa/dogfood` pin test. -->

Clause-②: no

The contract surface (`packages/spec`) is not in this diff; no authorable key, no closed-set member, no published export and no registry entry moves.

## What was wrong

#18066 gave this route ONE absence emitter and reached it from the two conditions that RETURN nothing. The conditions that THROW one were left on the classification door, which renders the flat envelope — a string `error` beside a top-level `code`. So `body.error.code` — the accessor #8013 settled on and objectui#4252 reads — was `undefined` on exactly those, and **which envelope a caller had to parse for an absence was decided by two things it cannot see**:

- `metadata.enableCache`, which **defaults to `true`**. The cached arm's `getMetaItemCached` throws `metadataItemNotFoundError` on a falsy `item`; the uncached arm resolves item-less and returns.
- which protocol implementation is mounted. The in-repo `metadata-protocol` resolves item-less from `getMetaItem`; a protocol that throws the miss reached the same flat door.

Re-measured on `origin/main` at `551139bb7` rather than copied from the report — the same absent `view`, driven through both arms:

| arm | status | body |
|:--|--:|:--|
| uncached, item-less return | 404 | `{"error":{"code":"RESOURCE_NOT_FOUND","message":"Metadata item not found or access denied."}}` |
| cached, producer throws | 404 | `{"error":"Metadata item view/no_such_view not found","code":"RESOURCE_NOT_FOUND"}` |

Same route, same status, same code, two envelopes — and the flat one echoed the type and the name where the emitter says one fixed sentence.

## What it does now

Both arms reach `sendMetaItemAbsent`. The route's absence answer is one body:

```
404 {"error":{"code":"RESOURCE_NOT_FOUND","message":"Metadata item not found or access denied."}}
```

⭐ This **strengthens** the ADR-0045 §3 property rather than merely preserving it. The unpublished app and the service-gated one already answered through the emitter, so an absence that kept the thrown dialect was a response pair that told them apart — by envelope shape, and by the producer's prose. Byte-identity across all of them is now pinned on the SERIALIZED body, not on object equality.

## **BREAKING** — the default wire answer moves for non-`app` types

**BREAKING** in the accept-set sense, landing in the launch window as `minor` (the lockstep convention: `major` is refused by `check-changeset-no-major`, and breaking-ness is carried by this banner plus the ADR-0087 disposition above).

What breaks: on `GET /meta/:type/:name`, the **absence** refusal moves from the flat top-level `code` to the nested `error.code`. ⚠️ For every type that does **not** bypass the cache — `object`, `view`, `flow`, `page` and the rest — this is the **default** answer, not a minority path: `metadata.enableCache` defaults to `true`, so those types took the cached arm and the cached arm threw. Measured in this repo against a real booted app: the showcase declares no `enableCache`, and its dogfood pin on `GET /meta/object/:name` was reading the flat `body.code` — a real consumer, in-tree, depending on the flat shape for exactly this refusal.

Only `app` (and `dashboard`, `doc`, `book`, `?state=draft`, `?preview=draft`, `?package=`) bypassed the cache and already answered the nested shape.

**The remedy is one accessor.** Read `body.error.code` instead of `body.code` on this route's 404. Nothing else about the refusal moves: the status is still `404`, the code is still `RESOURCE_NOT_FOUND`, and the message is the emitter's fixed sentence rather than the producer's. `ObjectStackClient` normalizes both envelopes already, so SDK callers are unaffected.

## ⛔ What it deliberately does NOT do

- **It is not "every 404 is absence."** `NO_DRAFT` is a 404 on this same route — the Studio designer's `?state=draft` probe — and it says the item **is** there and its draft is not. Folding it in would tell a designer the object does not exist: #5532's flattening, reintroduced by the repair for a sibling of it. A producer-declared code the ADR-0112 ledger does not know keeps its `declaredCode` for the same reason, and a producer that declared NO code gets none invented for it.
- **It does not converge the flat dialect itself.** That envelope POSITION is the live ratchet **#9559** owns repo-wide (`check:route-envelope`); converting two of `sendDeclaredFault`'s four emissions here would mint a new divergence — the same audience refusal answering two shapes depending on which ROUTE served it.
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,26 @@ describe('showcase: anonymous posture is uniform across surfaces (#2567)', () =>
}
const r = await stack.apiAs(adminToken, 'GET', `/meta/object/${META_PROBE_OBJECT}`);
expect(r.status, 'the object the anonymous PUT tried to author must not exist').toBe(404);
const body = (await r.json()) as Record<string, unknown>;
expect(body.code).toBe('RESOURCE_NOT_FOUND');
const body = (await r.json()) as { error?: { code?: string } };
// [#18402] ENVELOPE, not semantics. The claim this case makes — the
// anonymous PUT left nothing behind, so the object is absent — is carried
// by the `404` above and is unchanged; only where the code is READ moved.
// `GET /meta/:type/:name` used to answer absence in two envelopes and
// `metadata.enableCache` picked one, so this line read the FLAT `body.code`
// and the route's own item-less arm answered the nested one. Both arms now
// reach the single absence emitter, and this is the ADR-0112 accessor #8013
// settled on.
//
// ⭐ Worth recording where this file records it: the showcase declares no
// `enableCache`, so it runs the DEFAULT `true` and `object` takes the
// CACHED arm. This case is therefore the measurement that the flat dialect
// was the answer a default deployment really shipped for a non-`app` type —
// not the minority path.
//
// ⛔ Read in its own shape, with no `??` chain across the two shells — the
// #5632 rule this file already enforces for the 401 bodies and for the
// `/actions` 404 below.
expect(body.error?.code).toBe('RESOURCE_NOT_FOUND');
});

it('[#12176 D3] the retired compound save routes NOWHERE — 404 for everyone, not a 401', async () => {
Expand Down
64 changes: 64 additions & 0 deletions packages/rest/src/error-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2630,6 +2630,70 @@ export function logUnexpectedRouteError(error: any, resolved: { status: number;
logWithheldServerFault(error, resolved);
}

/**
* [#18402] Would the classification door answer this caught value with a
* BARE `404 RESOURCE_NOT_FOUND` — no code the producer chose, nothing else
* riding along?
*
* ## Why a predicate rather than a second reading of the error
*
* A handler that owns ONE absence answer has to recognise the absences its
* producers THROW, and the tempting spelling — `error?.status === 404 &&
* error?.code === 'RESOURCE_NOT_FOUND'` — is a second opinion about what a
* caught value means. It disagrees with this door on every shape the door
* classifies rather than reads: a producer that declares a status and no code,
* an unregistered spelling that {@link thrownCodeFields} demotes to
* `declaredCode` while deriving `code` from the status, a structured arm that
* owns its own envelope. Each disagreement is one arm of one route quietly
* answering a different body again — the exact class the caller was fixing.
*
* So this ASKS the door. `resolveErrorResponse` is the function that would
* have rendered the value one line later; reading its verdict means the
* handler's fork and the fallback it forks away from can never drift apart.
* The function is pure and this runs on an error path, so the second
* classification pass costs nothing worth naming — the same argument
* {@link resolveErrorResponse} already makes for its own `mapDataError`
* re-entry.
*
* ## ⛔ Why it is NOT "the status is 404"
*
* MEASURED, and the measurement is the reason this function has three
* conditions instead of one. `404` on a metadata route is not a synonym for
* "you get nothing": `metadata-protocol` throws `{ code: 'NO_DRAFT', status:
* 404 }` from the Studio designer's draft probe — pinned byte-for-byte in
* `rest-expected-error-logging.test.ts` and `rest-4xx-message-truncation.test.ts`
* — and that refusal says the ITEM is there and its DRAFT is not. Folding it
* into an absence would tell a designer the object does not exist while it
* plainly does: the #5532 flattening, one pair over, minted by the repair for
* a sibling of it.
*
* So the question is asked about the ANSWER, not the status:
*
* - `status` is 404, and
* - `code` is `RESOURCE_NOT_FOUND`, i.e. the producer named that member. ⚠️
* MEASURED: a producer that declares a 404 and NO code at all does not get
* one derived into its BODY — {@link thrownCodeFields} answers `{}`,
* ADR-0112's rule that nothing is invented for the half the producer did
* not name — so that answer is false here and keeps the shape it had.
* Folding it in would mean inventing the member the ADR declines to
* invent, and
* - no `declaredCode` sits beside it. Presence MEANS demotion (see
* `ApiErrorSchema`): the producer spelled a code the ledger does not know,
* and ADR-0112 keeps that spelling as the open, author-authored channel.
* Converting such an answer would delete the one field it exists to carry.
*
* ⚠️ This predicate does NOT decide what a route answers — it only recognises
* an answer. The 503 an unreadable metadata store throws (#5532) resolves to
* 503 and is false here, which is the distinction that must never be
* flattened.
*/
export function thrownAnswerIsBareNotFound(error: any, object?: string): boolean {
const resolved = resolveErrorResponse(error, object);
return resolved.status === 404
&& resolved.body?.code === 'RESOURCE_NOT_FOUND'
&& resolved.body?.declaredCode === undefined;
}

/**
* The single door a route catch block should use: resolve the response once,
* log it only if it is a real fault, then send it. Wire behaviour is identical
Expand Down
32 changes: 24 additions & 8 deletions packages/rest/src/meta-app-publish-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,28 +492,44 @@ describe('#8013 — by-name: a permission denial is REPORTED, absence still is n
expect(absent.statusCode).toBe(404);
});

it('criterion 3: …and the REJECTING producer shape reaches the same status and code', async () => {
it('criterion 3: …and the REJECTING producer shape reaches the same BODY, not merely the same code', async () => {
// The other producer shape this door must survive: a protocol
// implementation that REJECTS with a declared `RESOURCE_NOT_FOUND` /
// `status: 404` (`rest-meta-outage-vs-miss.test.ts` pins the rendering).
// ⚠️ Its body is the FLAT `{ error: '<message>', code }` that
// `resolveErrorResponse`'s declared-status passthrough produces, not the
// nested ADR-0112 envelope the in-route refusals emit — so this case
// asserts `body.code`, and the case above asserts `body.error.code`, on
// purpose. Both reach this route, so the criterion is stated against
// both rather than against one stub's.
//
// [#18402] This case used to read `missing.body?.code` while criterion
// 2 above read `body.error.code` — "on purpose", said the note that
// stood here, because the rejecting shape rendered the FLAT
// `{ error: '<message>', code }` and the resolving shape the nested
// ADR-0112 envelope. ⚠️ That IS the finding: one door, one absence, two
// envelopes, and which one a caller got depended on the protocol
// implementation and on `metadata.enableCache` — neither visible to the
// caller. Both arms now reach this route's single absence emitter.
//
// ⭐ So the criterion is STRENGTHENED rather than translated: the
// rejecting shape is compared against the UNPUBLISHED app as a whole
// body, which is what ADR-0045 §3 actually asks. A status-and-code
// assertion could never have carried that — the flat and the nested
// body agreed on both while differing everywhere a client looks.
const { rest, protocol } = setup([], GATED_APPS);
protocol.getMetaItem = vi.fn().mockRejectedValue(Object.assign(
new Error('Metadata item app/no_such_app not found'),
{ code: 'RESOURCE_NOT_FOUND', status: 404 },
));

const missing = await getItem(rest, 'no_such_app');
const unpublished = await getItem(setup(['manage_users'], GATED_APPS).rest, 'production_management');

expect(missing.statusCode).toBe(404);
expect(missing.body?.code).toBe('RESOURCE_NOT_FOUND');
expect(missing.body?.error?.code).toBe('RESOURCE_NOT_FOUND');
expect(missing.body?.code).toBeUndefined();
expect(missing.statusCode).not.toBe(403);
expect(JSON.stringify(missing.body ?? {})).not.toContain('PERMISSION_DENIED');

// The producer's sentence named the type and the name; the emitter's
// names nothing, and the unpublished app answers the emitter's.
expect(JSON.stringify(missing.body)).toBe(JSON.stringify(unpublished.body));
expect(JSON.stringify(missing.body ?? {})).not.toContain('no_such_app');
});

it('criterion 4: the LIST route is untouched — the app is absent, not flagged', async () => {
Expand Down
Loading
Loading