Skip to content

Commit 4d2008c

Browse files
fix(rest): GET /meta/:type/:name answers 404 RESOURCE_NOT_FOUND for a name with nothing behind it (#18395)
Fixes #18066 Clause-②: no — this PR puts no new key on a published payload. It makes `GET /meta/app/NAME` answer the `RESOURCE_NOT_FOUND` / 404 that `packages/rest/src/meta-app-publish-gate.test.ts` already declares and that `GetMetaItemResponseSchema` already implies by making `item` required — restoring a declared contract, which narrows rather than widens. Declared by the dispatching seat in claim `5693775028`. `RESOURCE_NOT_FOUND` is an already-published code and no new `ERROR_CODE_LEDGER` entry is introduced. ## What it does `GET /api/v1/meta/:type/:name` for a name with nothing behind it answered **200** with the declared envelope MINUS its `item` member. It now answers `404 RESOURCE_NOT_FOUND`. The reported body, re-measured on this tree against the route's own `responseSchema`: ``` GET /api/v1/meta/app/no_such_app_xyz 200 {"type":"app","name":"no_such_app_xyz","lock":"none","editable":true,"deletable":true,"resettable":false} GetMetaItemResponseSchema.safeParse(that body) success = false issues = [{"code":"invalid_type","expected":"nonoptional","path":["item"]}] ``` ## Why this is execution, not a design call The card's "Not measured, deliberately left open" section reads like an open question. It is not: three declarations in this repository already agreed with each other and against the live route, and the loudest of the three — the 200 — was the wrong one. Governing text, maintainer ruling 2026-09-10, quoted rather than translated: > 我们的项目以objectstack 协议为准,文档应该以实际实现为准。协议不正确的应该先修改协议。 > 声明而未兑现是实现缺口,补实现或退役,⛔ 不在消费端收窄。 1. **`GetMetaItemResponseSchema`** (`packages/spec/src/api/protocol.zod.ts`, this route's declared `responseSchema` per `rest-route-ledger.ts`) makes `item` a required member, while every genuinely-optional key beside it is spelled `.optional()`. Measured above. `packages/spec` was read only and is not edited by this PR. 2. **The CACHED arm of this same route already answered 404.** `getMetaItemCached` throws `metadataItemNotFoundError` on a falsy `item`. So which arm a request took decided whether absence was an error at all — `app`, `dashboard`, `doc`, `book`, `?state=draft`, `?preview=draft`, `?package=` and every `enableCache: false` deployment are diverted around the cache. That is the #5563 defect class, one member over. 3. **`packages/runtime/src/domains/meta.ts`** refuses the same item-less answer in its object branch — "only treat the lookup as a hit when `item` is really there" — and 404s. Cited as corroboration, not touched here; see Acceptance notes. ## Mechanism, re-verified on today's tree The card's reading (dated 2026-09-13) still holds. Both the 403 `PERMISSION_DENIED` and the 404 `RESOURCE_NOT_FOUND` sit inside `if (isAppType && visible)`, where `visible` is the document, so for a name that resolves to nothing the whole gate was skipped and the envelope fell through to `res.json`. Two things the card left open, now measured: - **It was never an `app` rule.** The fall-through sat in the shared tail of the uncached arm, below the per-type gates, so every type diverted around the cache reached it. `app` is what the report saw because that type bypasses the cache structurally; a `?state=draft`, `?preview=draft` or `?package=` read of any type met the same 200. Pinned across `dashboard`, `view`, `object`, `flow`, `page` and all three query-flag diversions. - **The cached arm does NOT agree — it was already correct, so absence was never the uncached arm's opinion alone.** Read at the source: `packages/metadata-protocol/src/protocol.ts:12084` `getMetaItemCached` delegates to `getMetaItem`, takes `result?.item`, and at `if (!item)` throws `metadataItemNotFoundError(type, name)` — `RESOURCE_NOT_FOUND`, `status: 404`. Exercised over the route as well, not only read: §3 drives the cached branch with a provider that answers that rejection and asserts `getMetaItemCached` was the verb called, then drives the same address through the uncached branch with `enableCache: false` and asserts the two now agree on status and code. The one remaining asymmetry between them is the envelope dialect, which this PR deliberately does not touch (see Acceptance notes). And one mechanism detail the card could not have seen: - ⚠️ **The contract broke at `JSON.stringify`, not in the producer.** `metadata-protocol`'s `getMetaItem` returns `{ type, name, item: undefined, lock, … }` for a miss. `item` is *present* holding `undefined`, and `z.unknown()` admits that, so the returned object PASSES `GetMetaItemResponseSchema`; serialization then drops the member and produces the body the schema rejects. A conformance probe written against the returned object rather than the wire bytes reports agreement — which is the trap, not a detail. Both halves are asserted in §4. ## Type scope: separable, and covered anyway — with the reading The card measured `app` alone. This change covers every type, so the reading that justifies it: **One handler serves every type.** `GET /meta/:type/:name` is registered once, at `packages/rest/src/rest-server.ts:6593`, and `:type` is a path parameter. Inside it the only type-specific arms are four booleans computed at lines 6702 (`isAppType`), 6780 (`isDashboardType`) and 6784 (`isAudienceGatedType`, which is `book` or `doc`), plus a `doc`-only locale collapse. The producer call is at 6972, `let visible = envelope?.item` at 6979, and the new absence check at 7005-7012 — above all of them. The fall-through to `res.json` that produced the item-less 200 is in the shared tail and reads no type at all. **So is per-type narrowing possible? Yes — and I did not take it.** The narrowing is spellable in one token: `if (isAppType && visible == null)`. Stated plainly so it can be judged as an over-scope if that is the call. My reason for covering every type: - The contract being restored is declared **per route**, not per type: `GetMetaItemResponseSchema` is this route's single `responseSchema` for every `:type`. - The **cached arm already 404s for every type**, so `app`-only narrowing would leave the divergence standing everywhere else and make it depend on a query flag: `GET /meta/view/no_such_view` would answer 404 plainly and an item-less 200 under `?state=draft`. That is the same configuration-dependent split this change exists to remove, preserved for every type but one. - There is no per-type declaration anywhere saying absence is a success for some types — the only type-specific behaviour on this route is gating, and gating runs only on documents that exist. ⇒ On this reading, "all types" is the narrowest fix that is actually correct, not a widening. It is the maintainer's call to disagree; narrowing is a one-token patch round. **The 27 pages the docs-drift check names are AFFECTED, not edited.** This PR edits zero files under `content/docs/` — the diff is 3 files in `packages/rest/src` plus one changeset. `scripts/docs-audit/affected-docs.mjs` reports "27 docs name something this change touched", of which 6 are the release-owned pages under `content/docs/releases/`. Those are read-only under AGENTS.md's Documentation Guardrails and are not touched here. Read for a false claim about this behaviour: none of the six states anything about the by-name absence answer; the only `/meta/:type/:name` mentions are `v17/17-3.mdx` lines 362-370, which document the section-segment URL collapse and remain correct. Nothing to file against them. The breadth of the affected set is a property of the anchor (`rest-server.ts` plus the `/meta` route), not evidence of a wider code change. ## The RBAC ordering, which is the sharp part The new check is ordered **ahead of every gate**, and that ordering is the security half of the change rather than a style choice. The three gates below it all read `&& visible`, so they are reachable only by a document that exists; a name that resolves to nothing can therefore never be converted into the 403 that #8013 reserves for an app the caller may not open. An existing-but-withheld app answers exactly what it answered before. It also closes an enumeration hole in the **other** direction, which the card did not name. ADR-0045 §3 makes an unpublished app *externally unobservable*, and #8013 states the contract as absence and nonexistence being indistinguishable. They were not: an unpublished app answered the 404 while a nonexistent name answered the 200, so the pair of responses reported which app names exist-but-are-unpublished. Both absence answers now come from one emitter, `sendMetaItemAbsent`, and are byte-identical by construction rather than by two hand-built literals agreeing. | case | before | after | |:---|:---|:---| | name resolves to nothing | **200** + item-less envelope | **404** `RESOURCE_NOT_FOUND` | | app exists, `requiredPermissions` not held | 403 `PERMISSION_DENIED` | 403 `PERMISSION_DENIED` (unchanged) | | app exists, `_unpublished` | 404 `RESOURCE_NOT_FOUND` | 404 `RESOURCE_NOT_FOUND` (unchanged) | | app exists, `requiresService` absent | 404 `RESOURCE_NOT_FOUND` | 404 `RESOURCE_NOT_FOUND` (unchanged) | | metadata store unreadable | 503 `SERVICE_UNAVAILABLE` | 503 `SERVICE_UNAVAILABLE` (unchanged) | | `book` exists, audience not held | 403 `PERMISSION_DENIED` | 403 `PERMISSION_DENIED` (unchanged) | | `book` name resolves to nothing | **200** + item-less envelope | **404** `RESOURCE_NOT_FOUND` | Both directions are pinned on a non-`app` type too, because neither the invariant nor the trap is app-scoped: ADR-0046 §6.7's audience gate is the other arm on this handler that converts a withheld-but-existing document into a refusal, and it is guarded by `&& visible` exactly as the app gate is. A check placed after it — or one that fired on a document that exists — would turn a gated book into an absence. ⚠️ Measured while writing that pin, and recorded rather than repaired: the audience arm emits through `sendDeclaredFault`, whose body is the flat `{ error: '…', code }`, while the app gate's 403 one screen up emits `sendEnvelopeError`'s nested `{ success: false, error: { code, message } }`. One handler, one code, two dialects — `body.error.code`, the accessor #8013 settled on, reads `undefined` on the audience arm. The pin asserts the shape the route really sends. Filed below with the matching 404 split. ## The fixture half `meta-app-publish-gate.test.ts`'s double answered `undefined` for an unknown name — a shape no deployment produces, and the reason its criterion-3 case passed over a stub that could not exhibit the defect the live provider had. It now answers what the live producer answers: the protection envelope wrapped around an `item: undefined`. The rejecting-producer case keeps its own override, so both producer shapes reach this route from that file. That case also gains its positive half. "Not the denial" was satisfied by a **success** just as well as by an absence, which is how the route answered an item-less 200 for years with every assertion in that suite green. ## Verification **Ablation** (fix committed first; mutation = delete the absence check; restore = `git checkout HEAD -- …`): ``` HEAD blob = a43dfeb anchor count before = 2 (the new arm + the #8013 arm) anchor count after = 1 (only the #8013 arm is left) on-disk mutated = 7ad23dec956361fff62204636a829afb1abc266b === MUTATED RUN exit=1 === Tests 8 failed | 29 passed (37) on-disk restored = a43dfeb restore proven: git diff HEAD empty AND hash == HEAD blob === RESTORED RUN exit=0 === Tests 37 passed (37) ``` Direction observed: **turns red**, 8 cases. Worth recording which cases did NOT move — the rejecting-producer case and "an absent name never becomes the 403" both stayed green under the mutation, which is precisely the card's complaint: a fixture that rejects, and a fence phrased as a negative, cannot see this defect. **Suites and gates**, all re-run at the final commit `78f881fe`: | run | result | |:---|:---| | `pnpm --filter @objectstack/rest test` | 192 files, **3215 passed**, 1 skipped | | `pnpm --filter @objectstack/rest typecheck` | exit 0 (`tsc --noEmit` + `check:test-typecheck`: test layer compiles, 0 errors) | | `pnpm --filter '@objectstack/rest^...' build` | exit 0 (dependency closure) | | `pnpm exec turbo run build` (all packages) | 72/72 successful | | `node scripts/pm/dispatch-gates.mjs --commands` → 62 families, each run, exit codes captured to disk before reading | **61 green, 1 pre-existing red** | | `node scripts/pm/dispatch-gates.mjs --ran` | 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN | | `pnpm lint` (whole repo, `eslint . --no-inline-config`) | exit 0 — run in full, not narrowed | The one red is `pnpm check:cross-package-test-inputs`, and it is **not this diff**. It reds on `packages/cli/test/init-created-files-summary.e2e.test.ts` descending `packages/spec/dist/`. Controlled in place: the four changed paths were restored to their `origin/main` content with the build state left untouched, and the gate produced the byte-identical finding at exit 1 (`sendMetaItemAbsent` occurrences on disk = 0 during the control, then every blob restored to its HEAD hash with `git diff HEAD` empty). Filed as a finding below. Two gates first reported `PREREQUISITE NOT MET` (exit 3) and one a shallow-clone refusal; all three were re-run after the missing prerequisite was supplied and are green — `check:dual-build-cjs-loads` and `check:type-check-debt` after the full build, `check-plugin-teardown-shape --self-test` after `git fetch --unshallow`. None is recorded as a failed measurement. ## Acceptance notes **To file** (three-class findings, not fixed here — filing is the triage seat's, with dedupe words): - *(b) contract violation.* `packages/runtime/src/domains/meta.ts` — the generic `:type/:name` branch returns `deps.success(data)` straight from `protocol.getMetaItem(...)`, so the same item-less envelope reaches its callers as a success on that door too. Its own sibling object branch in the same function explicitly refuses this shape and 404s, so the file disagrees with itself. Not repaired here: `packages/runtime/` carries another live claim in this batch, so the bounded in-place exemption fails on the no-other-claimant condition. Dedupe words: `runtime meta dispatcher`, `domains/meta.ts singularType`, `deps.success(data) item-less`, `getMetaItem envelope`, `GetMetaItemResponseSchema item required`. - *(a) reproducible defect.* `pnpm check:cross-package-test-inputs` reds on unmodified `main` whenever `packages/spec/dist` exists on disk — the gate walks that tree, so the verdict is a function of local build state. A checkout that has run `pnpm build` gets a red naming a test it never touched; one that has not gets a green. Repro: run it with and without `packages/spec/dist` present, same source. Dedupe words: `cross-package-test-inputs`, `init-created-files-summary.e2e`, `packages/spec/dist walk`, `declared glob reaches inside`, `build-state-dependent gate`. - *(b) contract violation.* One route, two 404 dialects. `GET /meta/:type/:name` answers absence as ADR-0112's nested `{ error: { code, message } }` from the in-route arms and as the flat `{ error: '…', code }` from the thrown arm (`resolveErrorResponse`'s declared-status passthrough, pinned in `rest-meta-outage-vs-miss.test.ts`), so which envelope a caller must parse depends on a server-side cache setting it cannot see — a client reading `body.error.code` gets `undefined` on the cached arm, which is the #7035 failure class. Deliberately NOT converged here: matching the flat shape would have broken the byte-identity above, which is a security property, and converging the thrown side has repo-wide blast radius. Pinned as-is in §3 of the new file so a future convergence is a deliberate edit. Dedupe words: `meta item 404 dialect`, `resolveErrorResponse flat envelope`, `ADR-0112 nested error.code`, `getMetaItemCached throw`, `cached arm body shape`. **Noted, not filed:** - noted, not filed: the uncached arm resolves the execution context twice for an `app` read — once hoisted for `readOrganizationId`, once inside the RBAC gate. It is memoised per request (a `WeakMap` keyed on the request object), so the cost is a map lookup, and the hoisted comment already says so. Successor: none — no PR and no person is routed to this file by it. ## Scope `packages/rest/src/` only, as the claim declared. Authored by the `os-dev` seat in session `session_01DvvamiacK328idtBYJBxV3` (https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3) — recorded in prose because editing this body over raw REST replaces the session-URL footer with the platform's bare one. `packages/spec/src/` was read and not edited. No new error code, no `ERROR_CODE_LEDGER` entry, no `package.json` touched (so no lockfile is owed), no governed surface in the diff. No `skills/**` path, so no line-budget reading is owed. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7358c1c commit 4d2008c

4 files changed

Lines changed: 576 additions & 10 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
`GET /api/v1/meta/:type/:name` answers `404 RESOURCE_NOT_FOUND` for a name with nothing behind it, instead of `200` carrying the declared envelope minus its `item` member (#18066).
6+
7+
Measured on a real server (`examples/app-showcase`, API 17.4.0, four absent names, all identical):
8+
9+
```
10+
GET /api/v1/meta/app/no_such_app_xyz
11+
200 {"type":"app","name":"no_such_app_xyz","lock":"none","editable":true,"deletable":true,"resettable":false}
12+
```
13+
14+
Two declarations in this repository already said otherwise, and this restores what they declare rather than deciding anything new. `GetMetaItemResponseSchema` — the route's own `responseSchema` — makes `item` a required member; parsing the body above against it fails `invalid_type` / `expected: 'nonoptional'` at `item`. And the **cached** arm of this same route has always answered this condition with `404 RESOURCE_NOT_FOUND`, because `getMetaItemCached` throws on a falsy `item`. Which arm a request took was deciding whether absence was an error at all — `app`, `dashboard`, `doc`, `book`, `?state=draft`, `?preview=draft`, `?package=` and every `enableCache: false` deployment are diverted around the cache.
15+
16+
- **Every type is affected, not only `app`.** The fall-through sat in the shared tail of the uncached arm, below the per-type gates. The report measured `app` because that type bypasses the cache structurally; a `?state=draft` or `?package=` read of any type reached the same 200.
17+
- ⚠️ **The break was at `JSON.stringify`, not in the producer.** `metadata-protocol`'s `getMetaItem` returns `{ type, name, item: undefined, lock, … }` for a miss — `item` is *present* holding `undefined`, which `z.unknown()` admits — so the returned object conforms and only the serialized body does not. A conformance probe written against the object rather than the wire bytes reports agreement.
18+
- **The permission denial is unchanged.** `403 PERMISSION_DENIED` for an app that exists and whose `requiredPermissions` the session lacks answers exactly as before: the new check is ordered ahead of every gate, and those gates are reachable only by a document that exists, so an absent name can never be converted into a denial. Enumerating app names through the 403 stays impossible.
19+
- **It also closes an enumeration hole in the other direction.** ADR-0045 §3 makes an unpublished app *externally unobservable*, and an unpublished app answered this 404 while a nonexistent name answered the 200 — so the pair of responses reported which app names exist-but-are-unpublished. Both absence answers now come from one emitter and are byte-identical.
20+
- **An unreadable metadata store is still `503`, never this 404.** That distinction is a producer-side throw and never reaches the new check.
21+
22+
⚠️ **For callers**: a probe that read "the call did not throw" as "this name resolves" now sees the 404 it should always have seen. A caller that read the item-less 200 as a create-vs-edit signal must read the status instead. The console side was already corrected independently (objectui#9262 reads both dialects as absence), so no first-party consumer depends on the old shape.

packages/rest/src/meta-app-publish-gate.test.ts

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,24 @@ function setup(perms: string[], apps: any[] = ALL_APPS, serviceExists?: (name: s
112112
const t = String(type ?? '');
113113
return t === 'app' || t === 'apps' ? JSON.parse(JSON.stringify(apps)) : [];
114114
}),
115+
// [#18066] The MISS answers what the live producer answers, not
116+
// `undefined`. `metadata-protocol`'s `getMetaItem` resolves the
117+
// protection envelope AROUND a `item: undefined` for a name it cannot
118+
// find — `lock` / `editable` / `deletable` / `resettable` come from
119+
// `resolveLockState` unconditionally — so `undefined` was a shape no
120+
// deployment produces. That mattered: the door read `envelope?.item`,
121+
// which is `undefined` for both, and then fell through to `res.json`
122+
// with the envelope, so the criterion-3 case below was passing over a
123+
// stub that could not exhibit the defect the live provider had. The
124+
// reject-path case further down keeps its own override, so both
125+
// producer shapes reach this route from this file.
115126
getMetaItem: vi.fn(async ({ name }: any) => {
116127
const found = apps.find((a: any) => a.name === name);
117-
return found ? { type: 'app', name, item: JSON.parse(JSON.stringify(found)) } : undefined;
128+
return {
129+
type: 'app', name,
130+
item: found ? JSON.parse(JSON.stringify(found)) : undefined,
131+
lock: 'none', editable: true, deletable: true, resettable: false,
132+
};
118133
}),
119134
findData: vi.fn().mockResolvedValue([]),
120135
};
@@ -447,15 +462,46 @@ describe('#8013 — by-name: a permission denial is REPORTED, absence still is n
447462
expect(missing.statusCode).not.toBe(403);
448463
expect(refusal(missing.body).code).not.toBe('PERMISSION_DENIED');
449464
expect(JSON.stringify(missing.body ?? {})).not.toContain('PERMISSION_DENIED');
465+
466+
// [#18066] The POSITIVE half, which this case did not state and
467+
// which is what let the route answer `200` with an item-less
468+
// envelope for years while every assertion above stayed green:
469+
// "not the denial" was satisfied by a SUCCESS just as well as by an
470+
// absence. ADR-0112 — `status` and `code`.
471+
expect(missing.statusCode).toBe(404);
472+
expect(refusal(missing.body).code).toBe('RESOURCE_NOT_FOUND');
473+
expect(missing.body?.item).toBeUndefined();
474+
expect(missing.body?.lock).toBeUndefined();
450475
}
451476
});
452477

453-
it('criterion 3: …and the real producer miss is still the 404 it has always been', async () => {
454-
// The fixture's `getMetaItem` answers `undefined` for an unknown name;
455-
// `metadata-protocol` REJECTS with a declared `RESOURCE_NOT_FOUND` /
456-
// `status: 404` (pinned in `rest-meta-outage-vs-miss.test.ts`). Both
457-
// reach this route, so the criterion is stated against the production
458-
// shape too rather than against the stub's alone.
478+
it('criterion 3: …and it is the SAME answer the unpublished app gets, byte for byte', async () => {
479+
// [#18066] ADR-0045 §3 makes an unpublished app externally
480+
// unobservable, and this suite's partition note states the contract as
481+
// absence and nonexistence being indistinguishable. Over the live
482+
// producer shape they were not: `production_management` answered this
483+
// 404 while `no_such_app` answered a 200 envelope, so the pair
484+
// enumerated which app names exist-but-are-unpublished. Compared as
485+
// whole bodies rather than field by field, because a single extra key
486+
// on either side is the entire signal.
487+
const unpublished = await getItem(setup(['manage_users'], GATED_APPS).rest, 'production_management');
488+
const absent = await getItem(setup(['manage_users'], GATED_APPS).rest, 'no_such_app');
489+
490+
expect(absent.statusCode).toBe(unpublished.statusCode);
491+
expect(absent.body).toEqual(unpublished.body);
492+
expect(absent.statusCode).toBe(404);
493+
});
494+
495+
it('criterion 3: …and the REJECTING producer shape reaches the same status and code', async () => {
496+
// The other producer shape this door must survive: a protocol
497+
// implementation that REJECTS with a declared `RESOURCE_NOT_FOUND` /
498+
// `status: 404` (`rest-meta-outage-vs-miss.test.ts` pins the rendering).
499+
// ⚠️ Its body is the FLAT `{ error: '<message>', code }` that
500+
// `resolveErrorResponse`'s declared-status passthrough produces, not the
501+
// nested ADR-0112 envelope the in-route refusals emit — so this case
502+
// asserts `body.code`, and the case above asserts `body.error.code`, on
503+
// purpose. Both reach this route, so the criterion is stated against
504+
// both rather than against one stub's.
459505
const { rest, protocol } = setup([], GATED_APPS);
460506
protocol.getMetaItem = vi.fn().mockRejectedValue(Object.assign(
461507
new Error('Metadata item app/no_such_app not found'),

0 commit comments

Comments
 (0)