diff --git a/.changeset/rest-package-routes-single-implementation.md b/.changeset/rest-package-routes-single-implementation.md new file mode 100644 index 0000000000..5c5e89069d --- /dev/null +++ b/.changeset/rest-package-routes-single-implementation.md @@ -0,0 +1,33 @@ +--- +"@objectstack/rest": minor +--- + +`GET /api/v1/packages`, `GET /api/v1/packages/:id` and `DELETE /api/v1/packages/:id` have one implementation: the runtime dispatcher's `/packages` domain. `@objectstack/rest`'s `registerPackageRoutes` no longer mounts its own copies of those three routes; it mounts `POST /api/v1/packages/publish` and nothing else. + +The two copies had already diverged, and a comment in the REST registrar claimed its copies shadowed the dispatcher's while on a stock boot they were never mounted at all (the registrar decided at registration time, before the `package` service had registered). One URL, one body, ruled on #14503. + +What changes on the wire, for a deployment whose composition really did reach the REST copies: + +- `GET /packages/:id` answers `{ success: true, data: }` — the installed-package row directly under `data`. FROM `data.package` TO `data`. There is no `{ package }` wrapper. +- The rows on `GET /packages` and the row on `GET /packages/:id` carry no `source: 'registry' | 'database' | 'both'` key. **Deliberately removed**, not ported: it had no reader outside the REST registrar's own tests — none in this repo's production code, the Console, the docs or the OpenAPI document, and the SDK declined to declare it twice on purpose. +- `GET /packages` and `GET /packages/:id` read the **installed** packages from the in-memory registry (`registry.getAllPackages()` / `registry.getPackage(id)`) and nothing else. The REST copies merged the durable `sys_packages` rows (`PackageService.list()` / `.get(id, version)`) into the registry set, so a package **published but not installed** was listed there and gettable there; on the surviving door it is neither. **Deliberately removed** with the routes, not silently dropped: the published-artifact store keeps its own surface (`POST /packages/publish` here, the marketplace browse elsewhere), and the family this door serves is the installed set. +- `?version=` is not read on `GET /packages/:id` or `DELETE /packages/:id`, so its repeated-parameter refusal (`400 VALIDATION_ERROR` on `?version=a&version=b`) is gone with it. **Deliberately removed**: the single implementation reads the installed package from the registry, and a version-scoped durable lookup was a behaviour only the REST copy had. The one in-tree sender is the SDK's `ScopedEnvironmentClient.packages.get(id, version?)`, whose binding is tracked on #12034. +- A missing package answers `404 RESOURCE_NOT_FOUND` with the message `Package '' not found` (the dispatcher's spelling) instead of `Package "" was not found.`. +- `DELETE /packages/:id` uninstalls the package (registry plus persisted metadata rows, `?keepData=true` to keep the object tables); the REST copy's version-scoped delete of a published artifact is gone. +- **The uninstall's tenancy width narrows.** The REST copy called `protocol.deletePackage({ packageId, allTenants: true })` — a package-wide uninstall across every tenant, the width #7705 case 4 pinned on purpose because that registrar had no organization to resolve. The surviving door calls `protocol.deletePackage({ packageId, organizationId?, keepData? })` with the organization it resolves for the caller (`resolveActiveOrganizationId`), so a `DELETE /packages/:id` that used to reach the REST copy now removes the package's metadata for the caller's active organization, not for all tenants. **Deliberately narrowed**, not silently dropped: one door, one width, and it is the width the dispatcher has always answered on every stock boot. +- **Capability refusals answer a different `error.code`.** On all three routes a caller holding neither `manage_metadata` (write) nor `studio.access` / `setup.access` (read) is refused with `403 PERMISSION_DENIED`. FROM `403 FORBIDDEN` TO `403 PERMISSION_DENIED`: the removed REST copies emitted `sendError(res, 403, 'FORBIDDEN', …)` explicitly, while the dispatcher's `requireManageMetadata` / `requireReadCapability` (`packages/runtime/src/domains/packages.ts`) call `deps.error(message, 403)` with no code and `packages/runtime/src/error-envelope.ts` derives one from the status — `standardErrorCodeForHttpStatus(403)` = `PERMISSION_DENIED`. **Same status, same message**: the two cohort messages ("Managing packages requires the `manage_metadata` capability." and "Reading packages requires the `studio.access` or `setup.access` capability.") are identical on both doors. Both codes are ADR-0112 standard members, so the envelope shape is unchanged; what moves is that a client branching on `err.code === 'FORBIDDEN'` for a package read or delete refusal stops matching on any composition that really did reach the REST copies. + +`POST /api/v1/packages/publish` is unchanged. + +Spec conformance on the surviving door is claimed for `GET /packages/:id` **only**: its `{ success, data: , meta }` is exactly `GetInstalledPackageResponseSchema` (`packages/spec/src/api/package-api.zod.ts`, `data: InstalledPackageSchema` bare). The other two routes do **not** match their declarations, and the REST copies did not either — this drift is **pre-existing, not introduced by this release**, and is carded on #16781: + +- `GET /packages` answers `{ packages, total }`, while `ListInstalledPackagesResponseSchema` requires `hasMore` (and declares `enabled` / `limit` / `cursor` inputs the door does not read). +- `DELETE /packages/:id` answers `{ success, registryRemoved, persisted }`, while `UninstallPackageApiResponseSchema` requires `packageId`. + +Nothing in this release changes either shape; with one implementation there is now exactly one thing to reconcile, and #16781 carries that reconciliation together with the `responseSchema` pins the runtime ledger rows for `packages.list` / `packages.uninstall` still lack. + +`GET /discovery` on the REST server now advertises `routes.packages` on every boot — the family base under which its publish route is mounted — instead of only when its own copy of the list route had been mounted at start. On a stock `objectstack serve` boot that copy never was (the `package` service registers after the REST plugin starts), so discovery omitted `routes.packages` while the dispatcher served the family; the SDK's convention fallback covered it. + +The three removed REST rows are gone from `REST_ROUTE_LEDGER`; the runtime route ledger carries the surviving routes. + +The environment-scoped mount (`/environments/:environmentId/packages…`) is served by the same dispatcher domain **only where the `@objectstack/hono` catch-all is mounted** (`createHonoApp`): the catch-all strips the environment prefix and hands the request to the domain. The dispatcher plugin's own explicit mounts (`plugin-hono-server`) register `/packages*` at the **unscoped** prefix only, and that plugin's sole route into the dispatcher (`setFallbackHandler`) serves declarative `apis:` endpoints, not domains. So a host composed as `plugin-hono-server` + the REST plugin with `enableProjectScoping: true` + the dispatcher plugin, **without** `createHonoApp`, had exactly one door for scoped package reads and deletes — the REST mirror this release removes — and after it has none: the scoped `GET /environments/:id/packages`, `GET /environments/:id/packages/:id` and `DELETE /environments/:id/packages/:id` answer the transport's plain 404 there. That composition is reachable from the open-core CLI when the standalone boot is skipped (`shouldBootWithLibrary()` false — any host config, or `OS_MODE=off`) and `api.enableProjectScoping` is forwarded verbatim. Every consumer population reachable from this repo is zero for the scoped mount (no in-repo production caller of `ScopedEnvironmentClient.packages.*`, no Console call to a scoped `/packages` URL); it is stated here so it is a known gap rather than a silent one. On a `plugin-hono-server` composition with `enableProjectScoping` and no `createHonoApp`, the scoped `/api/v1/environments/:id/packages[/:id]` routes have no door until #16781 lands (ruled C′ on #14503). diff --git a/content/docs/kernel/contracts/metadata-service.mdx b/content/docs/kernel/contracts/metadata-service.mdx index 3363abd419..83fb9300f6 100644 --- a/content/docs/kernel/contracts/metadata-service.mdx +++ b/content/docs/kernel/contracts/metadata-service.mdx @@ -414,15 +414,17 @@ const draft = await metadataService.get('object', 'opportunity'); ### REST Endpoints -The REST layer mounts package routes under `/api/v1/packages` and per-item metadata -routes under `/api/v1/meta`. Publishing a single metadata item's pending draft is done -via the `/meta/:type/:name/publish` route. +The package family under `/api/v1/packages` is served by the runtime dispatcher's +`/packages` domain — one implementation for the reads and the uninstall — with the +REST layer contributing only the marketplace publish route beside it; per-item +metadata routes live under `/api/v1/meta`. Publishing a single metadata item's +pending draft is done via the `/meta/:type/:name/publish` route. | Method | Path | Description | |:---|:---|:---| -| `POST` | `/api/v1/packages` | Publish a package (body: `{ manifest, metadata }`) | -| `GET` | `/api/v1/packages` | List all packages (registry + database) | -| `GET` | `/api/v1/packages/:id` | Get a specific package | -| `DELETE` | `/api/v1/packages/:id` | Delete a package | +| `POST` | `/api/v1/packages/publish` | Publish a package to the marketplace registry (body: `{ manifest, metadata }`) — the REST registrar's one route | +| `GET` | `/api/v1/packages` | List the installed packages (the in-memory registry; published-but-not-installed artifacts are not listed) | +| `GET` | `/api/v1/packages/:id` | Get an installed package — the bare row under `data`; a missing id answers `404 RESOURCE_NOT_FOUND`, message `Package 'ID' not found` | +| `DELETE` | `/api/v1/packages/:id` | Uninstall a package for the caller's organization (`?keepData=true` keeps the object tables) | | `POST` | `/api/v1/meta/:type/:name/publish` | Promote a metadata item's pending draft to live | | `POST` | `/api/v1/meta/:type/:name/rollback` | Restore a historical version as the live overlay | diff --git a/content/docs/permissions/permission-sets.mdx b/content/docs/permissions/permission-sets.mdx index a30308f003..ac4a4663ea 100644 --- a/content/docs/permissions/permission-sets.mdx +++ b/content/docs/permissions/permission-sets.mdx @@ -293,7 +293,9 @@ is what makes package uninstall well-defined — and enforced: uninstalling a package (`DELETE /api/v1/packages/:id`) revokes its own sets, their position/user bindings, and its pending audience-binding suggestions in the same request (no ghost grants); the uninstall response reports the revocation -under `cleanups`. Environment-authored sets and other packages' rows survive. +under `data.persisted.cleanups` (the dispatcher's `/packages` domain answers +the route, and it nests the protocol's uninstall report under `persisted`). +Environment-authored sets and other packages' rows survive. ## One authoritative store — the record is a projection (ADR-0094) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 1227e7a146..b977ac709c 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -169,7 +169,7 @@ The largest single consumer — **17 of the 106 sites**. | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `packages/metadata-core/src/meta-write-capability.ts#metaWriteCapabilityVerdict` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `packages/runtime/src/domains/actions.ts#handleActionsRequest`, `packages/runtime/src/domains/ai.ts#handleAIRequest`, `packages/runtime/src/domains/automation.ts#handleAutomationRequest`, `packages/runtime/src/domains/meta.ts#handleMetadataRequest`, `packages/runtime/src/domains/security.ts#handleSecurityRequest`, `packages/runtime/src/domains/packages.ts#handlePackagesRequest`, `packages/rest/src/external-datasource-routes.ts#registerExternalDatasourceRoutes`, `packages/rest/src/package-routes.ts#refusePackageRequest` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `packages/runtime/src/domains/mcp.ts#handleMcpRequest` | -| 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `packages/rest/src/package-routes.ts#refusePackageRequest` | +| 54 | Package REST route capability gate bypassed | rest | Get: a marketplace publish over REST (`POST /packages/publish`, the one route the REST registrar mounts since #14503) without `manage_metadata`; the package read cohort (`studio.access` / `setup.access`) is enforced by the dispatcher `/packages` domain's own read gate, where the reads are served | `packages/rest/src/package-routes.ts#refusePackageRequest` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `packages/runtime/src/domains/packages.ts#requireManageMetadata`, `#requireReadCapability` | | 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `packages/runtime/src/domains/activation-gate.ts#refuseUngrantedActivationWrite`, `#refuseUngrantedActivationAuthoring` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#refuseUnrelatedScreenRead` | diff --git a/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts b/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts index d34e04f115..611ebd1959 100644 --- a/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts +++ b/packages/qa/dogfood/test/authz-probe-blind-spot.census.ts @@ -101,7 +101,7 @@ // conclusion that gets re-derived from scratch otherwise: // // WHAT THE LEDGERS DO COVER — richly, and more than this table ever has. -// `packages/rest/src/rest-route-ledger.ts`: 94 audited rows over 19 families, +// `packages/rest/src/rest-route-ledger.ts`: 91 audited rows over 19 families, // every route `@objectstack/rest` mounts, enumerated through // `RestServer.getRoutes()` on a booted server and guarded per route by // `rest-route-ledger.conformance.test.ts`. It reaches all 17 registrars; @@ -277,18 +277,25 @@ export const PROBE_FILE_CENSUS: readonly ProbeFileReading[] = [ kinds: ['ROUTE_ENUMERATION'], probes: 1, keys: 19, - population: 94, - reachable: 94, + population: 91, + reachable: 91, blindSpot: 0, populationRule: 'ledger rows inside REST_ROUTE_LEDGER; reachable = rows carrying a `family` (each distinct value mints a key)', - controls: { "route: '": 94, "family: '": 94, RestRouteLedgerEntry: 2 }, + controls: { "route: '": 91, "family: '": 91, RestRouteLedgerEntry: 2 }, note: 'The audited disposition of every route @objectstack/rest mounts, enumerated through ' + 'RestServer.getRoutes() on a booted server and guarded per route by rest-route-ledger.conformance.test.ts. ' + 'That guard is why this file can be a population source and a regex table cannot: a mounted route with no ' + 'row here is already RED in another package, so a new family cannot be silently absent from this file, ' + 'and therefore cannot be silently absent from the authz ratchet either. 19 families; 1 classified by a ' + - 'matrix row (metadata), 18 enumerated in the shrink-only baseline.', + 'matrix row (metadata), 18 enumerated in the shrink-only baseline. Re-measured 94 -> 91 when the ' + + 'three REST package read/delete rows (GET /packages, GET /packages/:id, DELETE /packages/:id) left the ' + + 'ledger with their routes; each carried `family: packages`, so `reachable` moved with ' + + '`population` (91/91) and the blind spot stays 0 -- the family itself survives on the publish row.', + // The 94 -> 91 re-measurement above landed with #14503 (the REST registrar + // keeps only POST /packages/publish; the dispatcher domain is the single + // implementation of the reads and the delete). The id lives here, not in + // the string: a runtime string reaches readers who cannot resolve it. }, { file: 'packages/runtime/src/route-ledger.ts', diff --git a/packages/rest/src/direct-mount-base-follows-apipath.test.ts b/packages/rest/src/direct-mount-base-follows-apipath.test.ts index 985e80db6c..89d23f7e96 100644 --- a/packages/rest/src/direct-mount-base-follows-apipath.test.ts +++ b/packages/rest/src/direct-mount-base-follows-apipath.test.ts @@ -45,7 +45,7 @@ import { toTemplatePath } from './openapi-builtin-paths.js'; type Handler = (req: any, res: any) => any; -/** The ledger's own list of the nine, as `VERB {base-relative}` suffixes. */ +/** The ledger's own list of the six, as `VERB {base-relative}` suffixes. */ const DIRECT_MOUNT_SUFFIXES = REST_ROUTE_LEDGER .filter((e) => e.source === 'direct-mount') .map((e) => { @@ -179,7 +179,7 @@ describe('#6306 — with `apiPath` set, the direct-mount routes follow it', () = const API_PATH = '/backend/api/v9'; const config = { api: { api: { apiPath: API_PATH } } }; - it('mounts all nine under {apiPath}, and leaves nothing behind at the convention prefix', async () => { + it('mounts all six under {apiPath}, and leaves nothing behind at the convention prefix', async () => { const { table, expectedBase } = await bootPlugin(config); // The base is the server's, not a second expression that happens to agree. @@ -192,14 +192,14 @@ describe('#6306 — with `apiPath` set, the direct-mount routes follow it', () = ).toContain(`${method} ${expectedBase}${suffix}`); } - // The whole surface moved, not merely the nine: no route is left at the + // The whole surface moved, not merely the six: no route is left at the // `/api/v1` convention. This is the split itself — on `origin/main` this // set had exactly 9 members. const stragglers = mountedKeys(table).filter((k) => k.split(' ')[1].startsWith('/api/v1')); expect(stragglers, 'no route may stay at /api/v1 when apiPath moves the surface').toEqual([]); }); - it('documents all nine in {apiPath}/openapi.json — the filter that made the split visible now includes them', async () => { + it('documents all six in {apiPath}/openapi.json — the filter that made the split visible now includes them', async () => { const { table, expectedBase } = await bootPlugin(config); const doc = await serveOpenApi(table, expectedBase); @@ -224,9 +224,11 @@ describe('#6306 — with `apiPath` set, the direct-mount routes follow it', () = expect(discovery.routes.packages).toBe(`${expectedBase}/packages`); expect(discovery.routes.datasources).toBe(`${expectedBase}/datasources`); - const pkg = resolveRoute(table, 'GET', discovery.routes.packages); - expect(pkg, 'the advertised packages URL must be mounted').toBeDefined(); - // [#7033 / #7023] `GET /packages` is now authz-gated, and this real plugin + // [#14503] The advertised base is the family base; this registrar's one + // route, `POST {base}/packages/publish`, is what must be mounted under it. + const pkg = resolveRoute(table, 'POST', `${discovery.routes.packages}/publish`); + expect(pkg, 'the advertised packages base must carry the mounted publish route').toBeDefined(); + // [#7033 / #7023] `POST /packages/publish` is authz-gated, and this real plugin // boot wires the production caller resolver // (`RestServer.resolvePackageRouteExecutionContext`) with no auth service in // the ctx — so the anonymous discovery probe resolves to no identity and the @@ -255,11 +257,11 @@ describe('#6306 — with `apiPath` set, the direct-mount routes follow it', () = // --------------------------------------------------------------------------- describe('#6306 — the base is READ, not rebuilt: `??` and `||` no longer disagree', () => { - it('an empty `basePath` puts the nine where the rest of the surface already was', async () => { + it('an empty `basePath` puts the six where the rest of the surface already was', async () => { // A second, independent way the two expressions differed: the plugin // defaulted with `||` (empty string ⇒ `/api`) while `RestServer` // normalizes with `??` (empty string kept). So `basePath: ''` mounted the - // RouteManager surface at `/v1` and the nine at `/api/v1` — the same + // RouteManager surface at `/v1` and the six at `/api/v1` — the same // split, reached without `apiPath` at all. Reading the base cannot // disagree with itself. const { table, expectedBase } = await bootPlugin({ api: { api: { basePath: '', version: 'v1' } } }); @@ -284,7 +286,7 @@ describe('#6306 — default and conventional configs are unchanged', () => { // pinning that single-sourcing moved nothing for deployments that never set // `apiPath` (measured: the default mount list is identical, 92 routes, // before and after). - it('default config keeps all nine at /api/v1, documented and advertised there', async () => { + it('default config keeps all six at /api/v1, documented and advertised there', async () => { const { table, expectedBase } = await bootPlugin(undefined); expect(expectedBase).toBe('/api/v1'); diff --git a/packages/rest/src/direct-mount-composition.ts b/packages/rest/src/direct-mount-composition.ts index f83aab9705..472a8b72c0 100644 --- a/packages/rest/src/direct-mount-composition.ts +++ b/packages/rest/src/direct-mount-composition.ts @@ -22,24 +22,28 @@ * * - a route this boot mounted ⇒ it is enumerable through `getRoutes()` and * appears in `GET {apiPath}/openapi.json`; - * - a route this boot skipped (a package route needing a `package` service - * that is not there) ⇒ nothing is recorded, nothing is documented, and the - * 404 a caller would get from that deployment is what the document says too. + * - a route this boot skipped ⇒ nothing is recorded, nothing is documented, + * and the 404 a caller would get from that deployment is what the document + * says too. (No registrar takes that branch today — both mount + * unconditionally, see below — so it is the contract for the next one.) * * [#7563] That second bullet promised a 404 and, for `POST /packages/publish`, * did not get one: with no owner for the path, the dispatcher's * `/packages/:id` matched it (`id = "publish"`) and the router answered 405 * with THAT route's `Allow` set. The publish route therefore mounts on every - * boot and answers its own honest 404 — see `package-routes.ts` for why the - * other three must not follow it. + * boot and answers its own honest 404. [#14503] The three package routes that + * used to sit behind the `package`-service gate beside it (`GET /packages`, + * `GET /packages/:id`, `DELETE /packages/:id`) are gone from the registrar + * altogether — `packages/runtime`'s `/packages` domain is their single + * implementation — so the package registrar carries no service gate at all + * now, and neither does this step. * - * The service gate stays exactly where it was — around the package registrar's - * routes — and the record follows it rather than restating it. What is - * deliberately NOT recorded is any verdict about a service that a later phase - * could still contradict: the federation routes mount unconditionally and - * decide per request whether the `external-datasource` service is there (503 if - * not), so this file records them as mounted and says nothing about federation - * being available. + * What is deliberately NOT recorded is any verdict about a service that a + * later phase could still contradict: the federation routes mount + * unconditionally and decide per request whether the `external-datasource` + * service is there (503 if not), and the publish route resolves the `package` + * service per request (404 if none is composed), so this file records both as + * mounted and says nothing about either service being available. */ import type { PluginContext } from '@objectstack/core'; @@ -58,8 +62,6 @@ export interface DirectMountComposition { ctx: PluginContext; /** The configured API base, e.g. `/api/v1`. */ versionedBase: string; - /** The `protocol` slice the package routes read registry packages through. */ - protocol?: PackageRoutesOptions['protocol']; /** * [#7033 / #7023] Resolves the caller's execution context for the direct- * mount gates — the `RestServer`'s own resolver, so the checks read the @@ -86,7 +88,7 @@ export interface DirectMountComposition { * mounted on {@link DirectMountComposition.recorder}. */ export function mountAndRecordDirectRoutes(composition: DirectMountComposition): void { - const { server, recorder, ctx, versionedBase, protocol, resolveExecutionContext } = composition; + const { server, recorder, ctx, versionedBase, resolveExecutionContext } = composition; const enableProjectScoping = composition.enableProjectScoping ?? false; const projectResolution = composition.projectResolution ?? 'auto'; @@ -99,12 +101,12 @@ export function mountAndRecordDirectRoutes(composition: DirectMountComposition): // plugins with no edge between them, so asking once here answered "no // package service" on precisely the deployments that have one. // - // `registerPackageRoutes` decides what that resolver's answer means per - // route: `POST /packages/publish` mounts either way (nobody else serves it, - // and an unowned path is answered by a `/packages/:id` sibling's 405 - // instead of a 404 — #7563), the other three only when a service is there - // (they shadow live dispatcher twins). It reports back exactly what it - // mounted, so the record still follows the gate rather than restating it. + // `registerPackageRoutes` mounts `POST /packages/publish` either way + // (nobody else serves it, and an unowned path is answered by a + // `/packages/:id` sibling's 405 instead of a 404 — #7563) and, since + // #14503, nothing else: the three read/delete routes it used to gate on + // that resolver are served by the dispatcher's `/packages` domain alone. + // It reports back exactly what it mounted, so the record is the mount. const resolvePackageService = () => { try { return ctx.getService('package'); @@ -123,7 +125,7 @@ export function mountAndRecordDirectRoutes(composition: DirectMountComposition): : [versionedBase]; for (const base of bases) { recorder.recordDirectMountedRoutes( - registerPackageRoutes(server, resolvePackageService, base, { protocol, resolveExecutionContext }), + registerPackageRoutes(server, resolvePackageService, base, { resolveExecutionContext }), ); } ctx.logger.info('Package management routes registered'); diff --git a/packages/rest/src/direct-mount-introspection.test.ts b/packages/rest/src/direct-mount-introspection.test.ts index a25cf75cee..d5a6605370 100644 --- a/packages/rest/src/direct-mount-introspection.test.ts +++ b/packages/rest/src/direct-mount-introspection.test.ts @@ -3,7 +3,7 @@ /** * DIRECT-MOUNT ROUTES ARE ENUMERABLE — and only when they are mounted (#5822). * - * The nine routes `package-routes.ts` and `external-datasource-routes.ts` mount + * The six routes `package-routes.ts` and `external-datasource-routes.ts` mount * straight on the host `IHttpServer` used to be invisible to the server that * owns the surface: `RestServer.getRoutes()` reported `RouteManager`'s table * alone, so `GET {apiPath}/openapi.json` — which #5588 / PR #5821 made a @@ -19,11 +19,11 @@ * mounted ⇒ enumerable, and documented * not mounted ⇒ absent from both * - * The second direction has a real trigger: three of the four package routes are - * gated on the `package` service, so a deployment without it serves none of - * them — and must not document them. The federation registrar is NOT gated (it - * mounts always and answers 503 per request), so "mounted" is unconditional - * there and the document says so. + * The second direction had a real trigger when it was written: three of the + * then-four package routes were gated on the `package` service, so a + * deployment without it served none of them — and must not document them. + * The federation registrar is NOT gated (it mounts always and answers 503 per + * request), so "mounted" is unconditional there and the document says so. * * [#7563] `POST /packages/publish` joined the unconditional cohort, and for a * reason the second direction is about rather than an exception to it: leaving @@ -32,9 +32,17 @@ * `id = "publish"` and the router answered 405 built from THAT route's method * set. "Not mounted ⇒ absent from the document" is honest only while "not * mounted" also means "not answered"; where it cannot, the route mounts and - * 404s for itself. The three gated routes have dispatcher twins at their own - * patterns and so keep the original treatment — that split is pinned in - * `package-publish-mount.test.ts`. + * 404s for itself. + * + * [#14503] The three gated routes are GONE from the registrar — they + * duplicated the dispatcher's `/packages` domain at byte-identical patterns + * and the ruling made that domain the single implementation — so today no + * registrar takes the gated branch at all. The second direction is kept as + * the contract (a registrar that skips a route must not document it) and is + * pinned from the other side: the three former patterns are documented by + * NEITHER boot, with or without a `package` service, because this package no + * longer mounts them. `package-publish-mount.test.ts` pins the same fact at + * the registrar. * * Both are driven through the REAL composition: `mountAndRecordDirectRoutes` * for the server-level facts, and `createRestApiPlugin().start()` for the @@ -110,13 +118,22 @@ function createCtx(services: Record) { }; } -/** The ledger's own list of the nine, split by registrar. */ +/** The ledger's own list of the six, split by registrar. */ const LEDGER_DIRECT_MOUNT = REST_ROUTE_LEDGER.filter((e) => e.source === 'direct-mount').map((e) => e.route); const ALL_PACKAGE_ROUTES = LEDGER_DIRECT_MOUNT.filter((r) => r.includes('/packages')); /** [#7563] Mounted on every boot — no dispatcher twin to fall back to. */ const PUBLISH_ROUTE = 'POST /api/v1/packages/publish'; -/** The `package`-service-gated three, each shadowing a dispatcher twin. */ -const PACKAGE_ROUTES = ALL_PACKAGE_ROUTES.filter((r) => r !== PUBLISH_ROUTE); +/** + * [#14503] The three patterns this registrar used to mount behind the + * `package`-service gate. They are the dispatcher `/packages` domain's alone + * now (`packages/runtime/src/route-ledger.ts` carries their rows), so the + * REST ledger must not list them and no REST boot may document them. + */ +const FORMER_PACKAGE_TWINS = [ + 'GET /api/v1/packages', + 'GET /api/v1/packages/:id', + 'DELETE /api/v1/packages/:id', +]; const FEDERATION_ROUTES = LEDGER_DIRECT_MOUNT.filter((r) => r.includes('/external')); /** `VERB /path` for every route the server reports as mounted. */ @@ -207,7 +224,7 @@ describe('#5822 — a registrar describes exactly what it mounted', () => { // --------------------------------------------------------------------------- describe('#5822 — mounted direct-mount routes are enumerable and documented', () => { - it('getRoutes() reports all nine, marked as direct-mount', () => { + it('getRoutes() reports all six, marked as direct-mount', () => { const { rest } = bootWith({ package: packageServiceStub() }); const keys = mountedKeys(rest); for (const route of LEDGER_DIRECT_MOUNT) { @@ -220,7 +237,7 @@ describe('#5822 — mounted direct-mount routes are enumerable and documented', expect(rest.getRoutes().some((r) => r.source === 'route-manager')).toBe(true); }); - it('the openapi built-in section carries all nine, ledger row by ledger row', async () => { + it('the openapi built-in section carries all six, ledger row by ledger row', async () => { const { server } = bootWith({ package: packageServiceStub() }); const body = await serveOpenApi(server); for (const route of LEDGER_DIRECT_MOUNT) { @@ -228,15 +245,15 @@ describe('#5822 — mounted direct-mount routes are enumerable and documented', } // The registration's summary and tags travel with them, exactly as they do // for a RouteManager route. - const list = body.paths['/api/v1/packages'].get; - expect(list.summary).toBe('List packages (registry + published)'); - expect(list.tags).toEqual(['packages']); + const publish = body.paths['/api/v1/packages/publish'].post; + expect(publish.summary).toBe('Publish a package to the marketplace registry'); + expect(publish.tags).toEqual(['packages']); expect(body.paths['/api/v1/datasources/{name}/external/tables'].get.parameters.map((p: any) => p.name)) .toEqual(['name']); }); it('still publishes nothing the server does not mount', async () => { - // #5588's set relation, re-proven with the nine added: growing the document + // #5588's set relation, re-proven with the six added: growing the document // must not loosen the rule that produced it. const { rest, server } = bootWith({ package: packageServiceStub() }); const body = await serveOpenApi(server); @@ -264,12 +281,17 @@ describe('#5822 — mounted direct-mount routes are enumerable and documented', projectResolution: 'auto', }); - expect(mountedKeys(rest)).toContain('GET /api/v1/environments/:environmentId/packages'); + expect(mountedKeys(rest)).toContain('POST /api/v1/environments/:environmentId/packages/publish'); + // [#14503] The scoped mirror carries publish and nothing else: the three + // former twins are not mounted on the scoped base either. + expect(mountedKeys(rest)).not.toContain('GET /api/v1/environments/:environmentId/packages'); + expect(mountedKeys(rest)).not.toContain('GET /api/v1/environments/:environmentId/packages/:id'); + expect(mountedKeys(rest)).not.toContain('DELETE /api/v1/environments/:environmentId/packages/:id'); const unscoped = await serveOpenApi(server, '/api/v1'); const scoped = await serveOpenApi(server, '/api/v1/environments/:environmentId'); - expect(unscoped.paths['/api/v1/environments/{environmentId}/packages']).toBeUndefined(); - expect(scoped.paths['/api/v1/environments/{environmentId}/packages'].get).toBeDefined(); + expect(unscoped.paths['/api/v1/environments/{environmentId}/packages/publish']).toBeUndefined(); + expect(scoped.paths['/api/v1/environments/{environmentId}/packages/publish'].post).toBeDefined(); }); }); @@ -278,30 +300,36 @@ describe('#5822 — mounted direct-mount routes are enumerable and documented', // --------------------------------------------------------------------------- describe('#5822 — an unmounted registrar is reported by nothing', () => { - it('the publish row this file splits out is a row the ledger really has', () => { - // Without this, renaming the route in the ledger would quietly move it into - // PACKAGE_ROUTES and make the gated-cohort cases below assert the opposite - // of what they are named after. - expect(ALL_PACKAGE_ROUTES).toContain(PUBLISH_ROUTE); - expect(PACKAGE_ROUTES).toHaveLength(ALL_PACKAGE_ROUTES.length - 1); + it('the publish row is the ONLY package row the ledger has (#14503)', () => { + // The ledger must not carry the three former twins either: a row nobody + // mounts is exactly the phantom this file exists to refuse, and the + // conformance guard would flag it — this pins the shape from this side. + expect(ALL_PACKAGE_ROUTES).toEqual([PUBLISH_ROUTE]); + for (const route of FORMER_PACKAGE_TWINS) expect(LEDGER_DIRECT_MOUNT).not.toContain(route); }); - it('a boot without the `package` service enumerates and documents no service-backed packages route', async () => { - const { rest, server } = bootWith({}); - const keys = mountedKeys(rest); - for (const route of PACKAGE_ROUTES) { - expect(keys, `${route} is not mounted on this boot and must not be enumerable`).not.toContain(route); - } - // Not merely absent from the table: absent from the wire too — the gate - // itself is unchanged, this pins that the record follows it. - const mountedPaths = server.get.mock.calls.map((args: unknown[]) => args[0]); - expect(mountedPaths).not.toContain('/api/v1/packages'); - - const body = await serveOpenApi(server); - for (const route of PACKAGE_ROUTES) { - expect(documented(body, route), `${route} is not mounted but is documented`).toBe(false); - } - }); + for (const [label, services] of [ + ['without', {}], + ['WITH', { package: packageServiceStub() }], + ] as const) { + it(`a boot ${label} the \`package\` service neither enumerates nor documents the three former package twins (#14503)`, async () => { + const { rest, server } = bootWith(services); + const keys = mountedKeys(rest); + for (const route of FORMER_PACKAGE_TWINS) { + expect(keys, `${route} is the dispatcher's alone and must not be enumerable here`).not.toContain(route); + } + // Not merely absent from the table: absent from the wire too. + const mountedGets = server.get.mock.calls.map((args: unknown[]) => args[0]); + expect(mountedGets).not.toContain('/api/v1/packages'); + expect(mountedGets).not.toContain('/api/v1/packages/:id'); + expect(server.delete.mock.calls.map((args: unknown[]) => args[0])).not.toContain('/api/v1/packages/:id'); + + const body = await serveOpenApi(server); + for (const route of FORMER_PACKAGE_TWINS) { + expect(documented(body, route), `${route} is not mounted but is documented`).toBe(false); + } + }); + } it('…but publish IS mounted, enumerable and documented on that same boot (#7563)', async () => { // The counterpart the header explains: this route has no dispatcher twin, @@ -343,7 +371,7 @@ describe('#5822 — the REST plugin records what it mounts', () => { return { server, ctx }; } - it('publishes the nine when the package service is there', async () => { + it('publishes the six when the package service is there', async () => { const { server } = await bootPlugin({ package: packageServiceStub() }); const body = await serveOpenApi(server); for (const route of LEDGER_DIRECT_MOUNT) { @@ -356,6 +384,6 @@ describe('#5822 — the REST plugin records what it mounts', () => { const body = await serveOpenApi(server); for (const route of FEDERATION_ROUTES) expect(documented(body, route)).toBe(true); expect(documented(body, PUBLISH_ROUTE), `${PUBLISH_ROUTE} mounts unconditionally (#7563)`).toBe(true); - for (const route of PACKAGE_ROUTES) expect(documented(body, route)).toBe(false); + for (const route of FORMER_PACKAGE_TWINS) expect(documented(body, route)).toBe(false); }); }); diff --git a/packages/rest/src/discovery-advertised-direct-mounts.parity.test.ts b/packages/rest/src/discovery-advertised-direct-mounts.parity.test.ts index cdf0dc2773..71571ecec1 100644 --- a/packages/rest/src/discovery-advertised-direct-mounts.parity.test.ts +++ b/packages/rest/src/discovery-advertised-direct-mounts.parity.test.ts @@ -78,17 +78,26 @@ function resolveRoute(table: Map, method: string, url: string): } /** Drive one mounted handler the way an adapter would, capturing the body. */ -async function drive(entry: { handler: Handler; params: Record }, query: Record = {}) { +async function drive( + entry: { handler: Handler; params: Record }, + query: Record = {}, + reqBody: unknown = {}, +) { let body: any; let statusCode = 200; const res: any = { status: (c: number) => { statusCode = c; return res; }, json: (b: any) => { body = b; }, + header: () => res, + send: () => {}, }; - await entry.handler({ params: entry.params, query, body: {} }, res); + await entry.handler({ params: entry.params, query, body: reqBody, headers: {} }, res); return { statusCode, body }; } +/** A manifest the publish route accepts — the one route the package registrar mounts (#14503). */ +const PUBLISH_BODY = { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }; + /** * Boot the real composition. `withPackageService` gates the package registrar * exactly the way production is gated (the `package` kernel service); @@ -107,7 +116,7 @@ function boot(opts: { registry: { getObject: (_n: string) => undefined, getRegisteredTypes: () => [] }, }; const services = new Map( - opts.withPackageService === false ? [] : [['package', { list: async () => [] }]], + opts.withPackageService === false ? [] : [['package', { publish: async () => ({ success: true }) }]], ); const protocol = new ObjectStackProtocolImplementation(engine as any, () => services); const config: any = { @@ -122,7 +131,7 @@ function boot(opts: { rest.registerRoutes(); const ctx = { getService: (name: string) => { - if (name === 'package' && opts.withPackageService !== false) return { list: async () => [] }; + if (name === 'package' && opts.withPackageService !== false) return { publish: async () => ({ success: true }) }; if (name === 'external-datasource') { return { listRemoteTables: async (_n: string, _o: any) => [{ name: 'customers' }] }; } @@ -139,8 +148,8 @@ function boot(opts: { // production wires its caller resolver here (via // `RestServer.resolvePackageRouteExecutionContext`). This parity test pins // mounted ⇒ advertised route PLACEMENT, not authz, so it stubs a capable - // caller — the advertised `GET /packages` URL then ANSWERS 200 the way an - // authorized caller reaches it in production, keeping this test's subject + // caller — the advertised base's `POST /packages/publish` then ANSWERS 200 + // the way an authorized caller reaches it in production, keeping this test's subject // (does the advertised URL resolve and answer in the mounted table) intact. // The gate itself is pinned in `package-envelope.conformance.test.ts`. // [#9901] `manage_platform_settings` joins the set for the same reason: @@ -186,11 +195,18 @@ describe('[#6633] /discovery advertises the direct-mount surfaces where they are ).toBeDefined(); // …and the advertised URLs answer through the SAME mounted table. - const pkg = resolveRoute(table, 'GET', discovery.routes.packages); - expect(pkg, 'advertised routes.packages must be a mounted GET route').toBeDefined(); - const pkgAnswer = await drive(pkg!); + // + // [#14503] `routes.packages` is the FAMILY base. This registrar's one + // contribution to the family is `POST {base}/packages/publish`, so that is + // what must resolve under the advertised base; the list route the + // advertisement used to be keyed on is the dispatcher domain's alone now + // and is deliberately NOT mounted here. + const pkg = resolveRoute(table, 'POST', `${discovery.routes.packages}/publish`); + expect(pkg, 'advertised routes.packages must be the base of the mounted publish route').toBeDefined(); + const pkgAnswer = await drive(pkg!, {}, PUBLISH_BODY); expect(pkgAnswer.statusCode).toBe(200); expect(pkgAnswer.body?.success).toBe(true); + expect(resolveRoute(table, 'GET', discovery.routes.packages), 'the list route is not this registrar\'s to mount (#14503)').toBeUndefined(); const ext = resolveRoute(table, 'GET', `${discovery.routes.datasources}/pg_main/external/tables`); expect(ext, 'advertised routes.datasources must be the base of the mounted federation family').toBeDefined(); @@ -210,9 +226,9 @@ describe('[#6633] /discovery advertises the direct-mount surfaces where they are expect(discovery.routes.packages).toBe('/backend/api/v9/packages'); expect(discovery.routes.datasources).toBe('/backend/api/v9/datasources'); - const pkg = resolveRoute(table, 'GET', discovery.routes.packages); + const pkg = resolveRoute(table, 'POST', `${discovery.routes.packages}/publish`); expect(pkg).toBeDefined(); - expect((await drive(pkg!)).body?.success).toBe(true); + expect((await drive(pkg!, {}, PUBLISH_BODY)).body?.success).toBe(true); const ext = resolveRoute(table, 'GET', `${discovery.routes.datasources}/pg_main/external/tables`); expect(ext).toBeDefined(); @@ -220,7 +236,7 @@ describe('[#6633] /discovery advertises the direct-mount surfaces where they are // The convention paths are NOT mounted on this boot, so advertising them // would have been the lie the projection exists to prevent. - expect(resolveRoute(table, 'GET', '/api/v1/packages')).toBeUndefined(); + expect(resolveRoute(table, 'POST', '/api/v1/packages/publish')).toBeUndefined(); expect(resolveRoute(table, 'GET', '/api/v1/datasources/pg_main/external/tables')).toBeUndefined(); // [#6714] The email surface is NOT a direct mount — it registers at the @@ -232,15 +248,25 @@ describe('[#6633] /discovery advertises the direct-mount surfaces where they are expect(resolveRoute(table, 'POST', '/api/v1/email/send')).toBeDefined(); }); - it('not mounted ⇒ not advertised: a boot without the package service advertises no routes.packages', async () => { + it('mounted ⇒ advertised on a boot WITHOUT the package service too: publish mounts unconditionally, so the family base is advertised (#7563 / #14503)', async () => { const { table } = boot({ versionedBase: '/api/v1', withPackageService: false }); const discovery = await readDiscovery(table); - // The registrar was never called (same gate as production), so nothing is - // recorded and nothing is advertised — D12's other half, kept honest even - // though the PROTOCOL half would happily stay silent too (no `package` - // service in its registry either; the override is what guarantees it). - expect(Object.prototype.hasOwnProperty.call(discovery.routes, 'packages')).toBe(false); + // This case used to pin the opposite — "not mounted ⇒ not advertised" — + // on the premise that the registrar's own `GET /packages` copy was the + // family's surface and was gated on the service. Both halves of that + // premise are gone: the copy is removed (#14503, the dispatcher domain is + // the single implementation) and the registrar's one route mounts on + // every boot (#7563). What is advertised is what is mounted: the base + // under which publish sits, and publish answers its OWN 404 naming the + // surface rather than a sibling's 405 — the fact #7563 was filed for. + expect(discovery.routes.packages).toBe('/api/v1/packages'); + const publish = resolveRoute(table, 'POST', '/api/v1/packages/publish'); + expect(publish).toBeDefined(); + const answer = await drive(publish!, {}, PUBLISH_BODY); + expect(answer.statusCode).toBe(404); + expect(answer.body?.error?.code).toBe('RESOURCE_NOT_FOUND'); + expect(answer.body?.error?.message).toContain('marketplace publish surface'); expect(resolveRoute(table, 'GET', '/api/v1/packages')).toBeUndefined(); // The federation family mounts unconditionally (degrades per request), so @@ -281,8 +307,9 @@ describe('[#6633] /discovery advertises the direct-mount surfaces where they are const discovery = await readDiscovery(table, '/api/v1/environments/:environmentId', { environmentId: 'env_alpha' }); expect(discovery.routes.packages).toBe('/api/v1/environments/env_alpha/packages'); // The scoped variant is genuinely mounted (`auto` mirrors the package - // routes under both bases) and the advertised URL resolves against it. - expect(resolveRoute(table, 'GET', '/api/v1/environments/env_alpha/packages')).toBeDefined(); + // registrar under both bases) and the advertised base's publish resolves + // against it. + expect(resolveRoute(table, 'POST', '/api/v1/environments/env_alpha/packages/publish')).toBeDefined(); // The federation family has no scoped variant — the unscoped mount is the // truth, and the scoped response says so rather than inventing one. diff --git a/packages/rest/src/execctx-authz-input-seam-reachability.test.ts b/packages/rest/src/execctx-authz-input-seam-reachability.test.ts index 2c289b5a80..c40baf9d81 100644 --- a/packages/rest/src/execctx-authz-input-seam-reachability.test.ts +++ b/packages/rest/src/execctx-authz-input-seam-reachability.test.ts @@ -300,21 +300,28 @@ function mount(rest: RestServer): Map { delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, patch: () => {}, use: () => {}, listen: async () => {}, close: async () => {}, } as any; + // [#14503] The instrument is `POST /packages/publish` — the one route the + // registrar mounts now (the read routes it used to drive here are the + // dispatcher domain's alone). Same resolver, same gate, same seam; the write + // cohort (`manage_metadata`) is what the fixture's permission set grants. registerPackageRoutes( server, - () => ({ list: async () => [], publish: async () => ({}), delete: async () => ({}) }) as any, + () => ({ publish: async () => ({ success: true }) }) as any, '/api/v1', { resolveExecutionContext: (req: any) => rest.resolvePackageRouteExecutionContext(req) } as any, ); return routes; } +const PUBLISH_PATH = `${PKGS}/publish`; +const PUBLISH_BODY = { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }; + async function drive( routes: Map, headers: Record, ): Promise { - const handler = routes.get(`GET:${PKGS}`); - if (!handler) throw new Error(`no handler for GET ${PKGS}`); + const handler = routes.get(`POST:${PUBLISH_PATH}`); + if (!handler) throw new Error(`no handler for POST ${PUBLISH_PATH}`); const captured: Captured = { status: 0, body: undefined }; const res: any = { json(data: any) { captured.body = data; }, @@ -322,7 +329,7 @@ async function drive( status(code: number) { captured.status = code; return res; }, header() { return res; }, }; - await handler({ params: {}, query: {}, body: undefined, headers, method: 'GET', path: PKGS } as any, res); + await handler({ params: {}, query: {}, body: PUBLISH_BODY, headers, method: 'POST', path: PUBLISH_PATH } as any, res); return captured; } @@ -756,8 +763,8 @@ describe('[#15256] §3b — every computeExecCtx branch derives the posture', () headers: Record, params: Record, ): Promise { - const handler = routes.get(`GET:${PKGS}`); - if (!handler) throw new Error(`no handler for GET ${PKGS}`); + const handler = routes.get(`POST:${PUBLISH_PATH}`); + if (!handler) throw new Error(`no handler for POST ${PUBLISH_PATH}`); const captured: Captured = { status: 0, body: undefined }; const res: any = { json(data: any) { captured.body = data; }, @@ -765,7 +772,7 @@ describe('[#15256] §3b — every computeExecCtx branch derives the posture', () status(code: number) { captured.status = code; return res; }, header() { return res; }, }; - await handler({ params, query: {}, body: undefined, headers, method: 'GET', path: PKGS } as any, res); + await handler({ params, query: {}, body: PUBLISH_BODY, headers, method: 'POST', path: PUBLISH_PATH } as any, res); return captured; } diff --git a/packages/rest/src/package-delete-status-classification.test.ts b/packages/rest/src/package-delete-status-classification.test.ts deleted file mode 100644 index 37c6ef088f..0000000000 --- a/packages/rest/src/package-delete-status-classification.test.ts +++ /dev/null @@ -1,325 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [#8275] `DELETE /api/v1/packages/:id` answers a driver fault as a 5xx, and - * the caller's own errors as 4xx. - * - * ## The defect - * - * `packageService.delete` reported failure by RETURNING, so the handler - * answered `sendError(res, 400, 'PACKAGE_DELETE_FAILED', …)`. The statement - * that failed is `DELETE FROM sys_packages WHERE id = ? [AND version = ?]`, so - * a missing table, a lock timeout or a foreign-key restriction — a SERVER - * fault — was answered as a client error: it invited the caller to fix a - * request that was never the problem, and it hid a real fault from every - * dashboard that buckets by status. The mirror of what #8016 fixed on the - * throw path and #8131 fixed for `publish`, on the sibling route. - * - * ## What is DIFFERENT from the `publish` half, measured rather than inherited - * - * #8131 found that reclassifying `publish` was not enough on its own, because - * the returned failure was carrying `(error as Error).message` and the 5xx - * withhold (#8086) lives in `sendThrownError`, which a returned failure never - * reaches. The first half of that measurement holds here — §4 re-measures it - * on this route rather than assuming it — but the CONCLUSION does not carry - * over: this door builds its sentence from the request's own `:id` and - * `?version=`, and the producer returns a bare flag with no message channel at - * all. No driver text has ever reached a caller on this path, so there is - * nothing to withhold and no producer-side message to add. This card is a - * status-classification defect only. - * - * §4 pins that property where it can actually break: the door does not read a - * message off the producer's result, so a producer that grows one cannot put - * it on the wire through here. - * - * ## What is deliberately NOT asserted - * - * That a body "no longer contains" driver text, on its own — that passes on a - * route that emits nothing at all, including one whose handler never ran. - * Every case below asserts the POSITIVE shape (exact status, exact code, exact - * message) and, where a stub can say so, that the service was really reached. - */ - -import { describe, it, expect, vi } from 'vitest'; -import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { looksLikeInternalErrorLeak } from '@objectstack/types'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; -const PKG_ID = 'com.acme.crm'; - -/** The driver lines the real engine produced for this statement, measured. */ -const REAL_DRIVER_LINES = [ - 'no such table: sys_packages', - 'FOREIGN KEY constraint failed', -]; - -interface Captured { status: number; body: any } - -const CLEARS_THE_GATE = async () => ({ - userId: 'u_pkg', - systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], -}); - -function mount(svc: Record, options: Record = {}) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, use: () => {}, listen: async () => {}, close: async () => {}, - } as any; - registerPackageRoutes(server, () => svc as any, '/api/v1', { - resolveExecutionContext: CLEARS_THE_GATE, ...options, - } as any); - return routes; -} - -async function drive( - routes: Map, - method: string, - path: string, - req: Record = {}, -): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 0, body: undefined }; - const res: any = { - json(d: any) { captured.body = d; }, send() {}, - status(c: number) { captured.status = c; return res; }, header() { return res; }, - }; - await handler({ params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, res); - return captured; -} - -/** The declared envelope, imported from `packages/spec` rather than restated. */ -function expectDeclaredEnvelope(captured: Captured): any { - expect(BaseResponseSchema.safeParse(captured.body).success).toBe(true); - expect(envelopeViolations(captured.body)).toEqual([]); - expect(captured.body?.success).toBe(false); - const parsed = ApiErrorSchema.safeParse(captured.body?.error); - expect(parsed.error?.issues ?? []).toEqual([]); - expect(parsed.success).toBe(true); - return captured.body.error; -} - -// --------------------------------------------------------------------------- -// 1. A reported driver fault is a 5xx -// --------------------------------------------------------------------------- - -describe('[#8275] a returned delete failure answers 5xx, not 400', () => { - const SHAPES: Array<{ name: string; query: Record; message: string }> = [ - { - name: 'a version-scoped delete', - query: { version: '1.0.0' }, - message: `Failed to delete ${PKG_ID}@1.0.0.`, - }, - { - name: 'an unversioned delete with no protocol composed', - query: {}, - message: `Failed to delete ${PKG_ID}.`, - }, - ]; - - for (const shape of SHAPES) { - it(`${shape.name}: status AND code together (ADR-0112)`, async () => { - const del = vi.fn(async () => ({ success: false })); - - const captured = await drive( - mount({ delete: del }), 'DELETE', `${PKGS}/:id`, - { params: { id: PKG_ID }, query: shape.query }, - ); - - // The seam really ran — otherwise every assertion below is about a route - // that refused before reaching `delete`, which is a different answer. - expect(del, 'packageService.delete was never called').toHaveBeenCalledTimes(1); - - const error = expectDeclaredEnvelope(captured); - // ① the half that was mislabelled - expect(captured.status).toBe(500); - // ② the code is kept — it discloses nothing and says more than - // INTERNAL_ERROR. `envelopeViolations` imposes no code/status agreement, - // so a registered code on a 5xx is conformant. - expect(error.code).toBe('PACKAGE_DELETE_FAILED'); - // ③ the positive message shape, not merely "it changed" - expect(error.message).toBe(shape.message); - }); - } - - it('a successful delete is untouched', async () => { - const captured = await drive( - mount({ delete: async () => ({ success: true }) }), 'DELETE', `${PKGS}/:id`, - { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - expect(captured.status).toBe(200); - expect(captured.body?.success).toBe(true); - expect(captured.body?.data?.message).toBe(`Deleted ${PKG_ID}@1.0.0`); - }); -}); - -// --------------------------------------------------------------------------- -// 2. The caller's own errors are STILL 4xx — the over-block guard -// --------------------------------------------------------------------------- -// -// The ruling this card carries is that 4xx must not be swept. Without this -// section the change above is satisfied by "answer 500 for every delete -// failure", which would destroy the self-correcting messages #4277 exists for -// and re-break what #8016 fixed. - -describe('[#8275] a genuine CALLER error on this route is still 4xx', () => { - it('a REFUSAL thrown from below `delete` keeps its own status and code', async () => { - // The producer re-throws a declared envelope rather than swallowing it, so - // #8016's mapping answers. Before this change the swallow turned it into - // `{ success: false }` and the door answered `400 PACKAGE_DELETE_FAILED` — - // the producer's status AND code both lost. - const refusal = Object.assign(new Error('Uninstalling drops 3 tables; pass force: true.'), { - status: 409, code: 'DESTRUCTIVE_CHANGE', - }); - const captured = await drive( - mount({ delete: async () => { throw refusal; } }), 'DELETE', `${PKGS}/:id`, - { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - const error = expectDeclaredEnvelope(captured); - expect(captured.status).toBe(409); - expect(error.code).toBe('DESTRUCTIVE_CHANGE'); - // The self-correcting sentence survives verbatim — it names the remedy. - expect(error.message).toBe('Uninstalling drops 3 tables; pass force: true.'); - }); - - it('the `statusCode` spelling of a declared 4xx is answered too', async () => { - const refusal = Object.assign(new Error('[tenant_scope_required] pass organizationId.'), { - statusCode: 400, - }); - const captured = await drive( - mount({ delete: async () => { throw refusal; } }), 'DELETE', `${PKGS}/:id`, - { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - expect(captured.status).toBe(400); - expect(captured.body?.error?.message).toBe('[tenant_scope_required] pass organizationId.'); - }); - - it('a full uninstall that leaves items behind is STILL 400 PACKAGE_DELETE_PARTIAL', async () => { - // A DECLARED refusal on this route, and a different outcome from the one - // reclassified above: per-item failures are reported by the protocol, not - // by a broken statement. It must not be swept into the 5xx arm. - const captured = await drive( - mount({ delete: async () => ({ success: true }) }, { - protocol: { - deletePackage: async () => ({ - success: false, deletedCount: 1, failedCount: 2, - failed: [{ type: 'object', name: 'invoice', error: 'in use' }], - cleanups: [], - }), - }, - }), - 'DELETE', `${PKGS}/:id`, { params: { id: PKG_ID } }, - ); - const error = expectDeclaredEnvelope(captured); - expect(captured.status).toBe(400); - expect(error.code).toBe('PACKAGE_DELETE_PARTIAL'); - expect(error.details?.failed).toEqual([{ type: 'object', name: 'invoice', error: 'in use' }]); - }); - - it('a self-contradictory request is refused 400 before `delete` is reached', async () => { - const del = vi.fn(async () => ({ success: false })); - const captured = await drive( - mount({ delete: del }), 'DELETE', `${PKGS}/:id`, - { params: { id: PKG_ID }, query: { version: ['1.0.0', '2.0.0'] } }, - ); - // The refusal's other half: the service was never reached. A status - // assertion alone would not notice a handler that deleted anyway. - expect(del, 'delete ran on a request that should have been refused').not.toHaveBeenCalled(); - const error = expectDeclaredEnvelope(captured); - expect(captured.status).toBe(400); - expect(error.code).toBe('VALIDATION_ERROR'); - }); - - it('the 4xx/5xx split is decided by the CHANNEL, not by the message', async () => { - // The same package, failing the same way, once THROWN with a declared 4xx - // and once RETURNED. If the door ever starts sniffing text instead of - // reading the channel, this splits. - const sentence = `${PKG_ID}@1.0.0 could not be removed.`; - const thrown = await drive( - mount({ delete: async () => { throw Object.assign(new Error(sentence), { status: 422, code: 'VALIDATION_ERROR' }); } }), - 'DELETE', `${PKGS}/:id`, { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - expect(thrown.status).toBe(422); - expect(thrown.body?.error?.message).toBe(sentence); - - const returned = await drive( - mount({ delete: async () => ({ success: false }) }), - 'DELETE', `${PKGS}/:id`, { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - expect(returned.status).toBe(500); - }); -}); - -// --------------------------------------------------------------------------- -// 3. A declared 5xx from below is still the PRODUCER's answer -// --------------------------------------------------------------------------- - -describe('[#8275] a declared 5xx is not re-labelled by this route', () => { - it('a 503 SERVICE_UNAVAILABLE keeps its status and code', async () => { - // The re-throw is a test of DECLARATION, not of the status band. Answering - // this as `500 PACKAGE_DELETE_FAILED` would lose a code a client can - // branch on and a status that means something different (retry later). - const captured = await drive( - mount({ delete: async () => { throw Object.assign(new Error('The registry is warming up.'), { status: 503, code: 'SERVICE_UNAVAILABLE' }); } }), - 'DELETE', `${PKGS}/:id`, { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - const error = expectDeclaredEnvelope(captured); - expect(captured.status).toBe(503); - expect(error.code).toBe('SERVICE_UNAVAILABLE'); - }); -}); - -// --------------------------------------------------------------------------- -// 4. Why NO producer-side message was added here -// --------------------------------------------------------------------------- -// -// Recorded as executable fact rather than prose, because the shape of the -// `publish` fix makes "mirror it exactly" the obvious next edit, and on this -// route that edit would OPEN the channel it closed there. - -describe('[#8275] the door writes its own sentence and reads none from the producer', () => { - it('a producer that grows a message cannot put it on the wire through here', async () => { - // The structural pin. `sendError` applies no leak predicate at any status - // (#8086's withhold lives in `sendThrownError`, which a RETURNED failure - // never reaches), so anything the door chose to echo from the producer - // would travel unfiltered. It echoes nothing: the sentence is built from - // the request's own `:id` and `?version=`. - const leak = 'no such table: sys_packages'; - // The predicate would recognise this line — and is never asked on this - // path. That is the measurement, not a wish. - expect(looksLikeInternalErrorLeak(leak)).toBe(true); - - const captured = await drive( - mount({ delete: async () => ({ success: false, driverFault: { message: leak }, error: leak }) }), - 'DELETE', `${PKGS}/:id`, { params: { id: PKG_ID }, query: { version: '1.0.0' } }, - ); - - expect(captured.status).toBe(500); - expect(captured.body?.error?.message).toBe(`Failed to delete ${PKG_ID}@1.0.0.`); - for (const line of REAL_DRIVER_LINES) { - expect(JSON.stringify(captured.body)).not.toContain(line); - } - expect(JSON.stringify(captured.body)).not.toContain('sys_packages'); - }); - - it('the sentence echoes the request and nothing else', async () => { - // Two different requests, two different sentences, both derived only from - // what the caller sent — so the message channel is the request itself. - const first = await drive( - mount({ delete: async () => ({ success: false }) }), - 'DELETE', `${PKGS}/:id`, { params: { id: 'com.other.app' }, query: { version: '9.9.9' } }, - ); - expect(first.body?.error?.message).toBe('Failed to delete com.other.app@9.9.9.'); - - const second = await drive( - mount({ delete: async () => ({ success: false }) }), - 'DELETE', `${PKGS}/:id`, { params: { id: PKG_ID }, query: {} }, - ); - expect(second.body?.error?.message).toBe(`Failed to delete ${PKG_ID}.`); - }); -}); diff --git a/packages/rest/src/package-door-16019-raw-statement-fault-code.test.ts b/packages/rest/src/package-door-16019-raw-statement-fault-code.test.ts index b4a143aa79..5a4b6e86cd 100644 --- a/packages/rest/src/package-door-16019-raw-statement-fault-code.test.ts +++ b/packages/rest/src/package-door-16019-raw-statement-fault-code.test.ts @@ -1,18 +1,31 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#16019] `POST /api/v1/packages/publish` and `DELETE /api/v1/packages/:id` - * — the wire `code` a raw-exec driver fault answers moved, and this file pins - * the flip at the door. + * [#16019] `POST /api/v1/packages/publish` — the wire `code` a raw-exec driver + * fault answers moved, and this file pins the flip at the door. + * + * ## Scope, narrowed by #14503 + * + * This file arrived pinning BOTH published REST package doors. `DELETE + * /api/v1/packages/:id` is no longer one of them: #14503 ruled the dispatcher's + * `/packages` domain the single implementation of the package read and delete + * routes, so `registerPackageRoutes` mounts `POST /packages/publish` and + * nothing else and never calls `PackageService.delete`. The two `DELETE` cases + * are removed rather than re-pointed — the surviving door + * (`packages/runtime/src/domains/packages.ts`) uninstalls through + * `protocol.deletePackage` and the registry, never through + * `PackageService.delete`, so no delete-door subject for THIS producer is left + * in this package to pin. The producer-side half is untouched and still pinned + * where the catch lives: `service-package`'s `delete-driver-fault.test.ts` + * (`[#16019]` block). * * ## The flip * - * `PackageService.publish` / `delete` (`service-package/src/index.ts`) wrap + * `PackageService.publish` (`service-package/src/index.ts`) wraps * `objectql.execute(...)` in a catch whose branch ② re-throws any error that * `declaresHttpAnswer` — a numeric `status` or `statusCode` — and whose branch * ③ swallows everything else as a driver fault, returning `{ success: false }` - * for the door's `sendError` to answer `500 PACKAGE_PUBLISH_FAILED` / - * `500 PACKAGE_DELETE_FAILED`. + * for the door's `sendError` to answer `500 PACKAGE_PUBLISH_FAILED`. * * Before #16019 a raw-exec driver fault carried no `status` (knex's error * object: `code: 'SQLITE_ERROR'`, message `STATEMENT - DIAGNOSTIC`) → branch @@ -22,11 +35,11 @@ * → `500 DATABASE_ERROR`, the composed sentence as the message (it trips no * phrasing heuristic, so it is not replaced by `INTERNAL_ERROR_MESSAGE`; it * carries no dialect word to withhold). Same status band, no disclosure - * either way; the ledgered `code` on two published doors moves. + * either way; the ledgered `code` on the published door moves. * * The catch's own half — that the declared fault propagates UNCHANGED and the * undeclared ancestor still takes branch ③ — is pinned where the catch lives, - * in `service-package`'s `publish-driver-fault.test.ts` / + * in `service-package`'s `publish-driver-fault.test.ts` and * `delete-driver-fault.test.ts` (`[#16019]` blocks, identity-asserted). This * file takes the re-thrown object from there and pins what the DOOR answers, * with a `PackageService` double that throws it — the shape every @@ -133,11 +146,7 @@ async function publishWith(svc: Record): Promise { }); } -async function deleteWith(svc: Record): Promise { - return drive(mount(svc), 'DELETE', `${PKGS}/:id`, { params: { id: 'com.acme.crm' } }); -} - -describe('[#16019] a raw-exec driver fault under sys_packages answers the producer\'s code on both package doors', () => { +describe('[#16019] a raw-exec driver fault under sys_packages answers the producer\'s code on the publish door', () => { // The control that makes the assertions below about the DECLARATION and not // about the heuristic: the composed sentence trips nothing. it('the composed sentence is not a phrase the door\'s withhold heuristic knows', () => { @@ -168,28 +177,6 @@ describe('[#16019] a raw-exec driver fault under sys_packages answers the produc expect(error.code).toBe('PACKAGE_PUBLISH_FAILED'); }); - it('DELETE /packages/:id — AFTER #16019: the re-thrown declared fault → 500 DATABASE_ERROR, composed sentence, no dialect word', async () => { - const del = vi.fn(async () => { throw rawStatementFault(); }); - const captured = await deleteWith({ delete: del }); - - expect(del).toHaveBeenCalledTimes(1); - expect(captured.status).toBe(500); - const error = expectDeclaredEnvelope(captured); - expect(error.code).toBe('DATABASE_ERROR'); - expect(error.code).not.toBe('PACKAGE_DELETE_FAILED'); - expect(error.message).toBe(COMPOSED); - expect(JSON.stringify(captured.body)).not.toMatch(/sys_packages|no such table|insert into/i); - }); - - it('DELETE /packages/:id — BEFORE #16019: the swallowed driver fault → 500 PACKAGE_DELETE_FAILED (the control)', async () => { - const del = vi.fn(async () => ({ success: false })); - const captured = await deleteWith({ delete: del }); - - expect(captured.status).toBe(500); - const error = expectDeclaredEnvelope(captured); - expect(error.code).toBe('PACKAGE_DELETE_FAILED'); - }); - it('the withhold is untouched: a DECLARED fault whose message DOES carry dialect text is still replaced at this door', async () => { // Beside the flip, the invariant #8086 pinned: `sendThrownError` withholds // a leaky 5xx message whatever the code — so a producer that declared but diff --git a/packages/rest/src/package-door-5xx-message-sanitization.test.ts b/packages/rest/src/package-door-5xx-message-sanitization.test.ts index 58e170a581..680811a94c 100644 --- a/packages/rest/src/package-door-5xx-message-sanitization.test.ts +++ b/packages/rest/src/package-door-5xx-message-sanitization.test.ts @@ -19,9 +19,10 @@ * `resolveErrorResponse` at all. * * So one door of `/api/v1/packages` withheld a leaky 5xx and the other did - * not, on the same deployment — and this registrar is the one production - * serves for the routes both declare (first-match-wins, see the module note in - * `package-routes.ts`). + * not, on the same deployment. (The read/delete twins this registrar carried + * then are gone since #14503 — the dispatcher domain is their single + * implementation — so every case below drives the one route left, + * `POST /packages/publish`, for which this registrar is the only door.) * * This is option **B** of the three the card recorded, and the only one ruled: * apply the rule this surface already follows, at the door that was missed. @@ -35,15 +36,20 @@ * ## Reachability was MEASURED, not assumed * * The card was filed `Unverified`: grep proved the *filter was absent*, which - * is a different claim from the *leak being reachable*. Section 1 settles it by + * is a different claim from the *leak being reachable*. It was settled by * observation — a REAL `ObjectQL` engine, a REAL * `ObjectStackProtocolImplementation`, and a driver that fails every - * `sys_metadata` access the way a missing table does, driven through the route - * a client calls. The producer walked is `protocol.deletePackage`'s - * `engine.find('sys_metadata', …)`, which sits OUTSIDE that method's per-item - * `try`/`catch` and so propagates whole. + * `sys_metadata` access the way a missing table does, driven through the + * registrar's then-mounted `DELETE /api/v1/packages/:id`. The producer walked + * was `protocol.deletePackage`'s `engine.find('sys_metadata', …)`, which sits + * OUTSIDE that method's per-item `try`/`catch` and so propagates whole. That + * walk is no longer in this file: #14503 removed the delete route from this + * registrar, and the producer-side fact it had come to pin after #8136 (the + * protocol answers a declared 503 and quotes no driver text) is measured at + * the producer in `packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts` + * and at the surviving dispatcher door. * - * Before this change that request answered, verbatim: + * Before the withhold landed that request answered, verbatim: * * HTTP 500 * {"success":false,"error":{"code":"INTERNAL_ERROR", @@ -51,12 +57,12 @@ * * ## Reverse verification, direction predicted BEFORE running * - * Deleting the two-line withhold in `sendThrownError` turns the section-1 and - * section-2 leak cases RED — they assert the positive sanitized shape, so the - * driver line reappears in the diff — and leaves every pass-through case - * (section 3) and every 4xx case (section 4) GREEN, because the predicate is - * what decides and neither of those trips it. That is the ordinary direction, - * and it was confirmed by running it (quoted in the PR). + * Deleting the two-line withhold in `sendThrownError` turns the section-1 + * leak cases RED — they assert the positive sanitized shape, so the driver + * line reappears in the diff — and leaves every pass-through case (section 2) + * and every 4xx case (section 3) GREEN, because the predicate is what decides + * and neither of those trips it. That is the ordinary direction, and it was + * confirmed by running it (quoted in the PR). * * ## What is deliberately NOT asserted * @@ -71,8 +77,6 @@ import { describe, it, expect, vi } from 'vitest'; import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; import type { RouteHandler } from '@objectstack/spec/contracts'; -import { ObjectQL } from '@objectstack/objectql'; -import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { INTERNAL_ERROR_MESSAGE, looksLikeInternalErrorLeak } from '@objectstack/types'; import { registerPackageRoutes } from './package-routes.js'; @@ -80,7 +84,6 @@ const PKGS = '/api/v1/packages'; /** The driver line a missing `sys_metadata` produces on each dialect. */ const SQLITE_NO_TABLE = 'SQLITE_ERROR: no such table: sys_metadata'; -const PG_NO_RELATION = 'relation "sys_metadata" does not exist'; interface Captured { status: number; @@ -156,221 +159,23 @@ function thrown(message: string, carried: Record): Error { } // --------------------------------------------------------------------------- -// 1. The real producer, end to end — the card's "Unverified" half +// 1. Every catch site in this registrar, and the whole 5xx band // --------------------------------------------------------------------------- // -// Nothing is hand-built here: the engine, the protocol and the driver text all -// come from shipping code, and the route is the one a client calls. +// Two seams reach this registrar's one catch site (#14503 removed the +// read/delete routes and, with them, their `get` / `delete` / registry +// producer seams): the `publish` producer, and the capability-gate resolver. // -// `DELETE /api/v1/packages/:id` with no `?version=` routes to -// `protocol.deletePackage` (`package-routes.ts`, the `!version && typeof -// options.protocol?.deletePackage === 'function'` branch). That method's FIRST -// database touch is `this.engine.find('sys_metadata', { where })`, outside any -// `try` — its per-item `catch` only wraps the `deleteMetaItem` loop below it. -// So a driver failure on the overlay read propagates whole, out of the -// protocol, into this registrar's catch-all, and onto the wire. - -function failingDriver(dbError: string) { - const boom = () => { throw new Error(dbError); }; - const driver: any = { - name: 'memory-broken', version: '0.0.0', supports: {}, - async connect() {}, async disconnect() {}, async checkHealth() { return true; }, - async execute() { return null; }, - async find() { boom(); }, async findOne() { boom(); }, - async create() { boom(); }, async update() { boom(); }, async delete() { boom(); }, - async upsert() { boom(); }, async count() { boom(); }, - async bulkCreate() { boom(); }, async bulkUpdate() { boom(); }, async bulkDelete() { boom(); }, - async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, - async commit() {}, async rollback() {}, - }; - return driver; -} - -async function bootRealProtocol(dbError: string): Promise { - const engine = new ObjectQL(); - engine.registerDriver(failingDriver(dbError), true); - await engine.init(); - return new ObjectStackProtocolImplementation(engine as any); -} - -/** - * [#8136] **OPTION C LANDED, AND THIS SECTION IS ITS SIGNAL — INVERTED, NOT - * REPAIRED.** - * - * What used to open this block was an anti-vacuity guard asserting that - * `protocol.deletePackage` really does let the driver line out: - * - * ```ts - * await expect(protocol.deletePackage({ … })).rejects.toThrow(SQLITE_NO_TABLE); - * ``` - * - * It was written to go RED the day the producer stopped disclosing — "option C, - * the real cure" — so that a reader came back and re-read this section instead - * of consuming a green suite as proof the door was covered. #8136 is that day. - * Per the card's own instruction the pin is inverted rather than mended: making - * it green again would mean re-teaching the protocol to leak. - * - * So the subject of this section has moved by one layer, deliberately: - * - * before — "the producer emits a driver line and this DOOR withholds it" - * now — "the producer emits no driver line at all, and the envelope it - * does emit is DECLARED rather than guessed from a bare `Error`" - * - * The end-to-end walk is kept exactly as it was, because it is still the only - * thing here that proves the whole path: a real `ObjectQL`, a real - * `ObjectStackProtocolImplementation`, a driver that fails every `sys_metadata` - * access, driven through the route a client calls. What changed is what it - * observes at the far end. - * - * ⚠️ This does NOT retire the door's withhold, and section 2 onward still pins - * it in full. `sendThrownError` guards every producer that reaches this - * registrar, not just `metadata-protocol`, and #8131's `service-package` - * producer is a separate card still in flight. The belt stays; what changed is - * that this particular producer no longer needs it. - */ -describe('[#8136] a real sys_metadata failure, walked in process through this door', () => { - it('the producer no longer discloses: the driver line never leaves `deletePackage`', async () => { - // The inverted guard. This is the same call the old premise guard made, - // asserting the opposite fact — and it is still the anti-vacuity anchor for - // the section: if the protocol ever starts interpolating driver text again, - // this goes red at the source rather than the door silently covering for it. - const protocol = await bootRealProtocol(SQLITE_NO_TABLE); - - // The POSITIVE shape first, so this guard cannot pass vacuously — a bare - // `rejects.not.toThrow(...)` is green for a rejection with ANY other - // message, including a different leak, and green-by-accident is the exact - // failure mode this section exists to prevent. - await expect( - protocol.deletePackage({ packageId: 'com.acme.crm', allTenants: true }), - ).rejects.toMatchObject({ code: 'SERVICE_UNAVAILABLE', status: 503 }); - - await expect( - protocol.deletePackage({ packageId: 'com.acme.crm', allTenants: true }), - ).rejects.not.toThrow(SQLITE_NO_TABLE); - }, 60_000); - - it('the driver line does not appear anywhere in the client body', async () => { - const protocol = await bootRealProtocol(SQLITE_NO_TABLE); - - const captured = await drive( - mount({ delete: async () => ({ success: true }) }, { protocol }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - - const error = expectDeclaredEnvelope(captured); - // [#8136] The envelope is now the producer's own DECLARATION, not this - // door's guess. `metadata-protocol` answers an unreadable metadata store - // with 503 / `SERVICE_UNAVAILABLE` — the contract it already used for that - // exact condition — so the door forwards a declared refusal instead of - // falling to the undeclared-500 default and withholding its prose. - expect(captured.status).toBe(503); - expect(error.code).toBe('SERVICE_UNAVAILABLE'); - // Still the POSITIVE shape, not "it changed": the authored sentence, which - // is safe to ship precisely because it quotes nothing. - expect(error.message).toContain('The metadata store could not be read'); - expect(error.message).not.toBe(INTERNAL_ERROR_MESSAGE); - - const wire = JSON.stringify(captured.body); - expect(wire).not.toContain('SQLITE_ERROR'); - expect(wire).not.toContain('no such table'); - expect(wire).not.toContain('sys_metadata'); - }, 60_000); - - /** - * [#8132 → #8136] Twice-inverted, and the trail is the point. - * - * #8086 added this as a deliberately-red-in-future pin: the shared predicate - * knew no Postgres `relation … does not exist`, so that dialect's line - * travelled through this door while SQLite's was withheld. #8132 / #8263 - * closed that IN THE PREDICATE and the case flipped to "withheld too, by the - * shared predicate". #8136 now removes the disclosure at the producer, so - * there is nothing left for the predicate to decide about this path. - * - * ⚠️ The predicate assertion is KEPT, and deliberately still asserts `true` — - * it records that the interim belt is real and still standing for every other - * producer. What it no longer does is carry the weight of this path, and that - * is the structural difference option C bought: the body below is clean for a - * dialect the predicate has never met just as surely as for one it has. - * `packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts` - * measures that directly, across five dialects, three of which the predicate - * cannot see. - */ - it('the Postgres phrasing of the same failure is withheld at the producer now', async () => { - // The interim belt still exists and still recognises this phrasing. - expect(looksLikeInternalErrorLeak(PG_NO_RELATION)).toBe(true); - - const protocol = await bootRealProtocol(PG_NO_RELATION); - const captured = await drive( - mount({ delete: async () => ({ success: true }) }, { protocol }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - - const error = expectDeclaredEnvelope(captured); - // One door, one envelope, regardless of the engine underneath — the - // property the flip was always about, now held one layer earlier. - expect(captured.status).toBe(503); - expect(error.code).toBe('SERVICE_UNAVAILABLE'); - expect(error.message).toContain('The metadata store could not be read'); - - const wire = JSON.stringify(captured.body); - expect(wire).not.toContain('does not exist'); - expect(wire).not.toContain('sys_metadata'); - }, 60_000); -}); - -// --------------------------------------------------------------------------- -// 2. Every catch site in this registrar, and the whole 5xx band -// --------------------------------------------------------------------------- -// -// The live half above proves the door. These prove it is the DOOR and not one -// route: all four handlers exit through the same `sendThrownError`, so each is -// driven separately rather than assumed to share the fix. -// -// [#11063] `GET /packages` used to be different BY DESIGN — the sentence here -// read: "both of its data sources sit in their own inner `try { … } catch {}`, -// so nothing below reaches the outer catch". That is no longer true of the -// DURABLE source: #11063 removed its inner catch, because absorbing a failed -// durable read reported it as a 200 whose `total` claimed a complete count. A -// throw from `packageService.list()` now reaches this same outer catch and this -// same `sendThrownError`. -// -// This site is nevertheless left driving the GATE, deliberately: the resolver -// throw reaches the outer catch on this route regardless of what either data -// source does, so it exercises the CATCH SITE rather than one source — -// `refusePackageRequest` calls `options.resolveExecutionContext(req)`, and a -// resolver that throws SYNCHRONOUSLY throws before the -// `.catch(() => undefined)` is attached. The list door's durable-read arm is -// pinned separately in `package-list-durable-read-refusal.test.ts`. -// -// ⚠️ TEST-ONLY INJECTION POINT. This note used to end "so it keeps proving the -// DOOR rather than one source", which reads as a claim about a PRODUCTION path. -// It is not one: no production throw of any kind reaches this catch through -// this seam. What the site proves is how the door answers a SYNCHRONOUS gate -// throw — coverage of the catch site, never a claim about producers. ⛔ Do not -// read it as evidence that a production resolver can deliver a throw here. -// -// The derivation is stated ONCE — in the `Seam census` block of -// `package-door-declared-code.test.ts`, and the same conclusion is recorded in -// `package-door-user-message.test.ts`'s reachability section. Stable anchors: -// #12537, #12647. ⛔ It is deliberately NOT restated here. ⛔ The case is KEPT, -// not deleted: `reached()` keeps it from going vacuous, and its 5xx-withhold +// ⚠️ The resolver entry is a TEST-ONLY INJECTION POINT. No production throw of +// any kind reaches this catch through that seam: `refusePackageRequest` calls +// `options.resolveExecutionContext(req)`, and only a resolver that throws +// SYNCHRONOUSLY throws before the `.catch(...)` is attached. What the site +// proves is how the door answers a synchronous gate throw — coverage of the +// catch site, never a claim about producers. The derivation is stated ONCE, in +// the `Seam census` block of `package-door-declared-code.test.ts` (stable +// anchors: #12537, #12647), and is deliberately NOT restated here. The case is +// KEPT: `reached()` keeps it from going vacuous, and its 5xx-withhold // assertions still pin real door behaviour. -// -// ⚠️ [#11376] The sentence here used to add: "Still true of the REGISTRY -// source: `protocol.getMetaItems` keeps its own inner catch, which #11063 -// deliberately did not touch". Neither registry read in this registrar has one -// any more — the list door lost its catch in #11130, the detail door's in -// #11376, where the swallow was answering a terminal `404 RESOURCE_NOT_FOUND` -// for a read that could not happen. So EVERY data source in all four handlers -// now reaches the outer catch and this same `sendThrownError`, and this site -// keeps driving the GATE for the reason above rather than because the sources -// cannot get here. The two registry arms are pinned in -// `package-list-registry-read-refusal.test.ts` and -// `package-id-registry-read-refusal.test.ts`. interface Site { name: string; @@ -394,43 +199,18 @@ const SITES: Site[] = [ }, }, { - name: 'GET /packages — the capability gate resolver throws', + name: 'POST /packages/publish — the capability gate resolver throws', run: async (error: unknown) => { const resolveExecutionContext = vi.fn(() => { throw error; }); const captured = await drive( - mount({ list: async () => [] }, { resolveExecutionContext }), - 'GET', - PKGS, + mount({ publish: async () => ({ success: true }) }, { resolveExecutionContext }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); return { captured, reached: () => resolveExecutionContext.mock.calls.length === 1 }; }, }, - { - name: 'GET /packages/:id — packageService.get throws', - run: async (error: unknown) => { - const get = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ get }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => get.mock.calls.length === 1 }; - }, - }, - { - name: 'DELETE /packages/:id — packageService.delete throws', - run: async (error: unknown) => { - const del = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ delete: del }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => del.mock.calls.length === 1 }; - }, - }, ]; describe('[#8086] a leaky 5xx is withheld at every catch site, across the band', () => { @@ -502,7 +282,7 @@ describe('[#8086] a leaky 5xx is withheld at every catch site, across the band', }); // --------------------------------------------------------------------------- -// 3. The PREDICATE decides — not a blanket 5xx replacement +// 2. The PREDICATE decides — not a blanket 5xx replacement // --------------------------------------------------------------------------- // // Without this section the whole file is satisfied by `if (status >= 500) @@ -559,7 +339,7 @@ describe('[#8086] a 5xx that does NOT look like a leak passes through unchanged' }); // --------------------------------------------------------------------------- -// 4. The OVER-BLOCK guard: 4xx is untouched +// 3. The OVER-BLOCK guard: 4xx is untouched // --------------------------------------------------------------------------- // // A 4xx refusal's message is caller-facing BY DESIGN — it is the self-correcting @@ -635,7 +415,7 @@ describe('[#8086] a 4xx message is never withheld, even when it trips the predic }); // --------------------------------------------------------------------------- -// 5. #8016 must not regress +// 4. #8016 must not regress // --------------------------------------------------------------------------- // // This change rewrote the expression #8016 landed, so its half is re-pinned at @@ -644,12 +424,12 @@ describe('[#8086] a 4xx message is never withheld, even when it trips the predic describe('[#8086] the #8016 coded mapping still answers (non-regression)', () => { it('a coded 409 keeps its status, its code AND its message', async () => { - const { captured } = await SITES[3].run( - thrown('Uninstalling drops 3 tables', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), + const { captured } = await SITES[0].run( + thrown('Publishing would drop 3 tables', { status: 409, code: 'DESTRUCTIVE_CHANGE' }), ); expect(captured.status).toBe(409); expect(captured.body?.error?.code).toBe('DESTRUCTIVE_CHANGE'); - expect(captured.body?.error?.message).toBe('Uninstalling drops 3 tables'); + expect(captured.body?.error?.message).toBe('Publishing would drop 3 tables'); }); it('structured `details` survive the withhold on a leaky 5xx', async () => { diff --git a/packages/rest/src/package-door-declared-code.test.ts b/packages/rest/src/package-door-declared-code.test.ts index 1b776bc867..cd88d3bf4f 100644 --- a/packages/rest/src/package-door-declared-code.test.ts +++ b/packages/rest/src/package-door-declared-code.test.ts @@ -24,10 +24,10 @@ * * ## Why this door, and why it is a DISAGREEMENT rather than an omission * - * `/api/v1/packages` has two transports, and the module note in - * `package-routes.ts` records that this direct-mount registrar registers FIRST - * — so for the three routes both declare it is the one production serves. The - * twin, the runtime dispatcher domain (`packages/runtime/src/domains/packages.ts`), + * `/api/v1/packages` had two transports (since #14503 this registrar serves + * only `POST /packages/publish`, for which it is the one door; the read and + * delete twins it carried are the dispatcher domain's alone). The twin, the + * runtime dispatcher domain (`packages/runtime/src/domains/packages.ts`), * answers every catch through `errorFromThrown` * (`packages/runtime/src/http-dispatcher.ts`), which has emitted exactly this * channel since #9106: @@ -52,8 +52,8 @@ * into `PackageRoutesOptions`, and `resolveExecutionContext` is handed in by * the composition step. The door forwards whatever they throw, so the * demote fires on any spelling outside `@objectstack/spec`'s ledger. - * Section 1 drives all four seams through the real registrar — but - * ⚠️ only THREE of the four are production producers; the + * Section 1 drives both seams through the real registrar — but + * ⚠️ only ONE of the two is a production producer; the * `resolveExecutionContext` seam is a TEST-ONLY injection point. See * **Seam census** below, which is the one place that reason is stated. * @@ -61,19 +61,22 @@ * `pnpm check:dispatcher-error-vocabulary` (`dispatcher-error-vocabulary.ts`) * fails on an unswept platform producer precisely so a platform semantic * code cannot silently demote off the wire. Measured on this checkout: every - * status-declaring coded throw reachable at the three PRODUCTION seams + * status-declaring coded throw reachable at the PRODUCTION seam * (**Seam census** below) spells a REGISTERED code. That is the point of the gate, not an argument that the * channel is dead — it leaves `declaredCode`'s live population as the limb * no ledger can enumerate: a metadata app's own thrown `.code` across the * QuickJS boundary (#7867) and a downstream repo's codes, which the ledger's * federation ruling (2026-08-03/09) keeps out of this ledger BY DESIGN. * - * ## ⭐ Seam census: THREE production seams, plus ONE test-only injection + * ## ⭐ Seam census: ONE production seam, plus ONE test-only injection * - * `SITES` below drives FOUR seams. Three are producers a deployment can - * actually reach; the fourth is an injection point that exists only in a test. - * Stated ONCE here and cited from the sites that depend on it, rather than - * restated at each. + * `SITES` below drives TWO seams. One is a producer a deployment can actually + * reach — `packageService.publish`; the other is an injection point that + * exists only in a test. Stated ONCE here and cited from the sites that depend + * on it, rather than restated at each. (Until #14503 the table had four rows: + * `packageService.get` and `packageService.delete` reached this same catch + * through the read and delete routes, which the ruling removed — the + * dispatcher's `/packages` domain is their single implementation.) * * Measured on `origin/main` @ `aa5994e17` by reading the composition rather * than inferring it: @@ -154,13 +157,14 @@ * `package-routes.ts` site still carries none. So "deliberate" is established * by the code for the first site and still is not for the second. * - * Section 5 is the second fact stated as a test: a REAL `ObjectQL`, a REAL - * `ObjectStackProtocolImplementation` and a failing driver, driven through the - * route a client calls, answer a REGISTERED `SERVICE_UNAVAILABLE` and therefore - * carry NO `declaredCode`. It is this suite's proof that the instrument can say - * no on a real path — a suite that only ever asserted presence would be green - * for an implementation that stamped `declaredCode` on every refusal, which is - * the exact invariant `ApiErrorSchema.declaredCode` forbids. + * Section 5 used to state the second fact as a test: a REAL `ObjectQL`, a + * REAL `ObjectStackProtocolImplementation` and a failing driver, driven through + * `DELETE /packages/:id`, answered a REGISTERED `SERVICE_UNAVAILABLE` and + * therefore carried NO `declaredCode`. That route left this registrar with + * #14503, and the real-producer walk went with it: the producer-side fact is + * measured at the producer (`packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts`), + * and the surviving dispatcher door has its own pins. The instrument's "no" + * on a REGISTERED code is still asserted here, in section 2, on fixtures. * * ## Reverse verification, and the half the prediction got wrong * @@ -200,8 +204,6 @@ import { describe, it, expect, vi } from 'vitest'; import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; import type { RouteHandler } from '@objectstack/spec/contracts'; -import { ObjectQL } from '@objectstack/objectql'; -import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { resolveThrownHttpError, demotedDeclaredCode, @@ -287,12 +289,12 @@ const MANIFEST = { id: 'com.acme.crm', version: '1.0.0' }; /** * One catch site, plus the seam that drives a throw INTO it and a witness that - * the throw really travelled that way. Same four seams as + * the throw really travelled that way. Same two seams as * `package-routes-coded-error-mapping.test.ts`, for the same reason: a case * that silently never reached the seam would otherwise "pass" on a body it got * for a completely different reason. * - * ⚠️ Four seams, THREE of them production. The + * ⚠️ Two seams, ONE of them production. The * `resolveExecutionContext` entry is the test-only one — see **Seam census** * in the module docblock, the single place that reason is stated. */ @@ -324,48 +326,23 @@ const SITES: Site[] = [ // ⚠️ The `vi.fn` below is deliberately NOT `async`: an `async` one // would REJECT, and `package-routes.ts:81` would swallow that into the // 401 anonymous-deny floor instead of reaching `sendThrownError`. - name: 'GET /packages — the capability gate resolver throws', + name: 'POST /packages/publish — the capability gate resolver throws', run: async (error: unknown) => { const resolveExecutionContext = vi.fn(() => { throw error; }); const captured = await drive( - mount({ list: async () => [] }, { resolveExecutionContext }), - 'GET', - PKGS, + mount({ publish: async () => ({ success: true }) }, { resolveExecutionContext }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); return { captured, reached: () => resolveExecutionContext.mock.calls.length === 1 }; }, }, - { - name: 'GET /packages/:id — packageService.get throws', - run: async (error: unknown) => { - const get = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ get }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => get.mock.calls.length === 1 }; - }, - }, - { - name: 'DELETE /packages/:id — packageService.delete throws', - run: async (error: unknown) => { - const del = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ delete: del }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => del.mock.calls.length === 1 }; - }, - }, ]; // --------------------------------------------------------------------------- // 1. The demote reaches the wire, at every seam this suite drives -// (three production producers + one test-only injection — Seam census) +// (one production producer + one test-only injection — Seam census) // --------------------------------------------------------------------------- describe('[#12405] an UNREGISTERED producer spelling rides `declaredCode`', () => { @@ -522,7 +499,7 @@ describe('[#12405] `details` and `declaredCode` travel together', () => { * producer shape. */ it('a demoted refusal carrying structured context keeps BOTH', async () => { - const del = vi.fn(async () => { + const publish = vi.fn(async () => { throw thrown('two records still reference this package', { status: 409, code: 'CLOSE_PERIOD_LOCKED', @@ -530,13 +507,13 @@ describe('[#12405] `details` and `declaredCode` travel together', () => { }); }); const captured = await drive( - mount({ delete: del }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, + mount({ publish }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); - expect(del.mock.calls.length, 'the throwing seam was never called').toBe(1); + expect(publish.mock.calls.length, 'the throwing seam was never called').toBe(1); const error = expectDeclaredEnvelope(captured); expect(captured.status).toBe(409); expect(error.code).toBe('RESOURCE_CONFLICT'); @@ -547,21 +524,21 @@ describe('[#12405] `details` and `declaredCode` travel together', () => { }); it('a refusal with context but a REGISTERED code keeps `details` and gains nothing', async () => { - const del = vi.fn(async () => { - throw thrown('uninstalling drops 3 tables', { + const publish = vi.fn(async () => { + throw thrown('publishing would drop 3 tables', { status: 409, code: 'DESTRUCTIVE_CHANGE', issues: [{ path: 'tables', message: 'crm_account would be dropped' }], }); }); const captured = await drive( - mount({ delete: del }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, + mount({ publish }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); - expect(del.mock.calls.length, 'the throwing seam was never called').toBe(1); + expect(publish.mock.calls.length, 'the throwing seam was never called').toBe(1); const error = expectDeclaredEnvelope(captured); expect(error.details).toEqual({ issues: [{ path: 'tables', message: 'crm_account would be dropped' }], @@ -594,20 +571,19 @@ describe('[#12405] the demote is not withheld by the 5xx message sanitiser', () * merely consistent, because of a fact about the producers, and that fact is * recorded here because it can rot and is written down nowhere else: * - * **No producer reaching the three PRODUCTION seams can put a driver - * errno in `declaredCode` today, because `PackageService` discriminates on - * the STATUS channel and never on `.code`.** (Three, not four — **Seam - * census** in the module docblock.) + * **No producer reaching the PRODUCTION seam can put a driver errno in + * `declaredCode` today, because `PackageService` discriminates on the + * STATUS channel and never on `.code`.** (One, not two — **Seam census** + * in the module docblock.) * * `packages/services/service-package/src/index.ts` is explicit about why: * `publish` and `delete` re-throw only what `declaresHttpAnswer(error)` * accepts — a declared `status`/`statusCode` — and its own comment states the * reason in as many words, that "every SQL driver populates a string `code` * on its errors, so reading it would re-throw genuine driver faults as if - * they were refusals". `get` and `list` re-throw only the branded - * seam-unreadable refusal (`SERVICE_UNAVAILABLE` / 503). `protocol.deletePackage` - * escapes only with `TENANT_SCOPE_REQUIRED` or `metadataStoreUnavailableError`. - * So a bare `SQLITE_ERROR` / `42P01` is swallowed and re-answered long before + * they were refusals". (Its `get` / `list` / `delete` siblings and + * `protocol.deletePackage` used to reach this door too, through the routes + * #14503 removed.) So a bare `SQLITE_ERROR` / `42P01` is swallowed and re-answered long before * this door sees it, and the dialect-disclosure shape — a backend's own * error class landing in `declaredCode` beside a withheld message — is * unreachable rather than tolerated. @@ -619,11 +595,10 @@ describe('[#12405] the demote is not withheld by the 5xx message sanitiser', () * here measures that shape, because nothing produces it. * * ⛔ **The falsifier, stated so the next reader inherits a measurement - * instead of an argument:** a producer that reaches any of the three - * PRODUCTION seams carrying a driver errno as its `.code` — a - * `PackageService` implementation that re-throws on `.code` rather than on - * status, or a `protocol` slice that lets a raw driver error out of - * `deletePackage`. On that day the disclosure becomes live, this block's + * instead of an argument:** a producer that reaches the PRODUCTION seam + * carrying a driver errno as its `.code` — a `PackageService` + * implementation whose `publish` re-throws on `.code` rather than on + * status. On that day the disclosure becomes live, this block's * premise is false, and the fork has to be RE-OPENED rather than re-derived * from the consistency half above. * @@ -723,61 +698,3 @@ describe('[#12405] the wire `declaredCode` IS the shared rule, not a second copy expect(answers.filter((a) => a === undefined).length).toBeGreaterThan(2); }); }); - -describe('[#12405] a REAL producer walked through this door answers with NO demote', () => { - /** - * The instrument's "no", on a real path rather than a fixture — and the - * reachability measurement's second half stated as a test. - * - * A real `ObjectQL`, a real `ObjectStackProtocolImplementation` and a driver - * that fails every `sys_metadata` access, driven through - * `DELETE /api/v1/packages/:id` (no `?version=`, which is the branch that - * routes to `protocol.deletePackage`). The producer answers its own declared - * `503 SERVICE_UNAVAILABLE` — a REGISTERED code — so nothing is demoted and - * this door must add nothing. - * - * That is the framework-producer population in one case: platform producers - * reaching here are kept inside the ledger by - * `pnpm check:dispatcher-error-vocabulary`, which is why `declaredCode`'s - * live population is the limb no ledger enumerates (a metadata app's own - * `.code`, #7867; a downstream repo's codes, kept out of this ledger by the - * federation ruling). A suite that only ever asserted presence would be green - * for a door that stamped `declaredCode` on this body too. - */ - function failingDriver(dbError: string) { - const boom = () => { throw new Error(dbError); }; - const driver: any = { - name: 'memory-broken', version: '0.0.0', supports: {}, - async connect() {}, async disconnect() {}, async checkHealth() { return true; }, - async execute() { return null; }, - async find() { boom(); }, async findOne() { boom(); }, - async create() { boom(); }, async update() { boom(); }, async delete() { boom(); }, - async upsert() { boom(); }, async count() { boom(); }, - async bulkCreate() { boom(); }, async bulkUpdate() { boom(); }, async bulkDelete() { boom(); }, - async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, - async commit() {}, async rollback() {}, - }; - return driver; - } - - it('an unreadable metadata store answers 503 SERVICE_UNAVAILABLE with no `declaredCode`', async () => { - const engine = new ObjectQL(); - engine.registerDriver(failingDriver('SQLITE_ERROR: no such table: sys_metadata'), true); - await engine.init(); - const protocol = new ObjectStackProtocolImplementation(engine as any); - - const captured = await drive( - mount({ delete: async () => ({ success: true }) }, { protocol }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - - const error = expectDeclaredEnvelope(captured); - // The POSITIVE shape first, so the absence below cannot pass vacuously on - // a request that failed for some entirely different reason. - expect(captured.status).toBe(503); - expect(error.code).toBe('SERVICE_UNAVAILABLE'); - expect('declaredCode' in error).toBe(false); - }, 60_000); -}); diff --git a/packages/rest/src/package-door-execctx-fault-reachability.test.ts b/packages/rest/src/package-door-execctx-fault-reachability.test.ts index 7f4fa27de6..a649ae1a6b 100644 --- a/packages/rest/src/package-door-execctx-fault-reachability.test.ts +++ b/packages/rest/src/package-door-execctx-fault-reachability.test.ts @@ -65,8 +65,10 @@ * to an empty-but-valid envelope, answered 403. "Unresolvable" and * "unwired" are now two answers; section 3 drives both side by side. * - **Is a fault ever served as ANONYMOUS ACCESS, or as a silent success? NO.** - * Every degraded class is REFUSED on every wire-reachable method of all - * four routes. The swallow fails CLOSED. (Section 6.) + * Every degraded class is REFUSED on the route this registrar mounts + * (`POST /packages/publish` — the read and delete routes this file also + * drove before #14503 are the dispatcher domain's alone now). The swallow + * fails CLOSED. (Section 6.) * - **Does a fault ever reach the caller as the 5xx it is?** It did not, in * any class — that zero was read against a WORKING instrument: section 1 * shows this same door answering **500 `INTERNAL_ERROR`** when the fault is @@ -150,6 +152,15 @@ import { registerPackageRoutes } from './package-routes.js'; import { RestServer } from './rest-server.js'; const PKGS = '/api/v1/packages'; +/** + * [#14503] The instrument is `POST /packages/publish` — the one route the + * registrar mounts now; the read and delete routes this file used to drive + * beside it are the dispatcher domain's alone. Same resolver, same gate, same + * production wiring; `drive` supplies a valid publish body by default so a + * cleared gate reaches a 200 rather than a body-validation 400. + */ +const PUBLISH_PATH = `${PKGS}/publish`; +const PUBLISH_BODY = { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }; // --------------------------------------------------------------------------- // Harness — the REAL supplier and the REAL registrar, wired the production way. @@ -186,11 +197,11 @@ function serverWith(w: Wiring): RestServer { interface Captured { status: number; body: any } /** - * Mount the four package routes against a real `RestServer`, with the resolver + * Mount the package route against a real `RestServer`, with the resolver * wired EXACTLY as `rest-api-plugin.ts` wires it: * `resolveExecutionContext: (req) => restServer.resolvePackageRouteExecutionContext(req)`. */ -function mount(rest: RestServer, list: () => Promise = async () => []): Map { +function mount(rest: RestServer, publish: () => Promise = async () => ({ success: true })): Map { const routes = new Map(); const server = { get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, @@ -201,7 +212,7 @@ function mount(rest: RestServer, list: () => Promise = async () => []): } as any; registerPackageRoutes( server, - () => ({ list, publish: async () => ({}), delete: async () => ({}) }) as any, + () => ({ publish }) as any, '/api/v1', { resolveExecutionContext: (req: any) => rest.resolvePackageRouteExecutionContext(req) } as any, ); @@ -224,7 +235,7 @@ async function drive( header() { return res; }, }; await handler( - { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, + { params: {}, query: {}, body: path === PUBLISH_PATH ? PUBLISH_BODY : undefined, headers: {}, method, path, ...req } as any, res, ); return captured; @@ -272,22 +283,22 @@ const healthy = (): Wiring => ({ authServiceProvider: AUTH_OK, objectQLProvider: // --------------------------------------------------------------------------- describe('[#13255] controls — the instrument can produce 200, 401, 403 and 500', () => { - it('CONTROL 200: the full production stack, healthy end to end, serves the read', async () => { - const captured = await drive(mount(serverWith(healthy())), 'GET', PKGS); + it('CONTROL 200: the full production stack, healthy end to end, serves the publish', async () => { + const captured = await drive(mount(serverWith(healthy())), 'POST', PUBLISH_PATH); expect(captured.status).toBe(200); expect(captured.body?.success).toBe(true); }); it('CONTROL 200: and the capabilities came from the SHIPPED aggregation, not a stub', async () => { const rest = serverWith(healthy()); - const ctx = await rest.resolvePackageRouteExecutionContext({ params: {}, headers: {}, method: 'GET', path: PKGS }); + const ctx = await rest.resolvePackageRouteExecutionContext({ params: {}, headers: {}, method: 'POST', path: PUBLISH_PATH }); expect(ctx?.userId).toBe('u_admin'); expect(ctx?.systemPermissions).toContain('manage_metadata'); expect(ctx?.systemPermissions).toContain('studio.access'); }); it('CONTROL 401: a genuinely anonymous caller (no auth wired at all) is refused', async () => { - const captured = await drive(mount(serverWith({})), 'GET', PKGS); + const captured = await drive(mount(serverWith({})), 'POST', PUBLISH_PATH); expect(captured.status).toBe(ANONYMOUS_DENY_STATUS); expect(captured.body?.error?.code).toBe(ANONYMOUS_DENY_CODE); }); @@ -295,8 +306,8 @@ describe('[#13255] controls — the instrument can produce 200, 401, 403 and 500 it('CONTROL 403: an authenticated caller who genuinely holds nothing is refused on capability', async () => { const captured = await drive( mount(serverWith({ authServiceProvider: AUTH_OK, objectQLProvider: async () => qlEmpty() })), - 'GET', - PKGS, + 'POST', + PUBLISH_PATH, ); expect(captured.status).toBe(403); expect(captured.body?.error?.code).toBe('FORBIDDEN'); @@ -304,7 +315,7 @@ describe('[#13255] controls — the instrument can produce 200, 401, 403 and 500 it('⭐ CONTROL 500: THIS door does answer a 5xx — when the fault is raised by the package service', async () => { const routes = mount(serverWith(healthy()), async () => { throw new Error('driver exploded'); }); - const captured = await drive(routes, 'GET', PKGS); + const captured = await drive(routes, 'POST', PUBLISH_PATH); expect(captured.status).toBe(500); expect(captured.body?.error?.code).toBe('INTERNAL_ERROR'); }); @@ -443,7 +454,7 @@ describe('[#13255] reachability — each production fault class, driven, with it it.each(CLASSES)('$id — $what', async (klass) => { // ---- the fault ------------------------------------------------------- const rest = serverWith(klass.faulted()); - const req = { params: {}, headers: {}, method: 'GET', path: PKGS, ...(klass.req ?? {}) }; + const req = { params: {}, headers: {}, method: 'POST', path: PUBLISH_PATH, ...(klass.req ?? {}) }; // [#13279] The loud cohort never produces a context to inspect — that IS // the repair. The resolution REJECTS with the branded outage error instead // of fabricating an envelope that reports a capability set nobody read. @@ -457,7 +468,7 @@ describe('[#13255] reachability — each production fault class, driven, with it expect(isAuthzStoreUnavailableError((settled as any).e)).toBe(true); expect((settled as any).e.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); // POSITIVE CONTROL, unchanged: the same wiring minus the fault is served. - const served = await drive(mount(serverWith(healthy())), 'GET', PKGS, klass.req ?? {}); + const served = await drive(mount(serverWith(healthy())), 'POST', PUBLISH_PATH, klass.req ?? {}); expect(served.status).toBe(200); return; } @@ -476,7 +487,7 @@ describe('[#13255] reachability — each production fault class, driven, with it // ---- POSITIVE CONTROL: the same wiring, fault removed ---------------- // A refusal above is caused by the injected fault, not by a harness that // could never have been served in the first place. - const control = await drive(mount(serverWith(healthy())), 'GET', PKGS, klass.req ?? {}); + const control = await drive(mount(serverWith(healthy())), 'POST', PUBLISH_PATH, klass.req ?? {}); expect(control.status).toBe(200); expect(control.body?.success).toBe(true); }); @@ -488,18 +499,10 @@ describe('[#13255] reachability — each production fault class, driven, with it // --------------------------------------------------------------------------- describe('[#13255] consequence — the door\'s answer for each fault class', () => { - it.each(CLASSES)('$id — read and write cohorts', async (klass) => { + it.each(CLASSES)('$id — the write cohort, on the one route left', async (klass) => { const routes = mount(serverWith(klass.faulted())); const extra = klass.req ?? {}; - const read = await drive(routes, 'GET', PKGS, extra); - expect(read.status).toBe(klass.read.status); - expect(read.body?.error?.code).toBe(klass.read.code); - - const del = await drive(routes, 'DELETE', `${PKGS}/:id`, { ...extra, params: { ...(extra.params ?? {}), id: 'com.acme.crm' } }); - expect(del.status).toBe(klass.write.status); - expect(del.body?.error?.code).toBe(klass.write.code); - const publish = await drive(routes, 'POST', `${PKGS}/publish`, { ...extra, body: { manifest: { id: 'com.acme.crm', version: '1.0.0' } }, @@ -523,8 +526,6 @@ describe('[#13255] consequence — the door\'s answer for each fault class', () const routes = mount(serverWith(klass.faulted())); const extra = klass.req ?? {}; const bucket = klass.ctx === 'loud' ? loud : quiet; - bucket.push((await drive(routes, 'GET', PKGS, extra)).status); - bucket.push((await drive(routes, 'DELETE', `${PKGS}/:id`, { ...extra, params: { ...(extra.params ?? {}), id: 'x' } })).status); bucket.push((await drive(routes, 'POST', `${PKGS}/publish`, { ...extra, body: { manifest: { id: 'x', version: '1.0.0' } } })).status); } // The ruled class: the outage is the answer, on EVERY route — not one door @@ -556,10 +557,10 @@ describe('[#13255] consequence — the door\'s answer for each fault class', () // holds nothing" is TRUE. A supported shape; stays quiet. // - FAILED — the engine was wired and could not be resolved. "This // caller holds nothing" is UNKNOWN, and was being asserted. - const unwired = await drive(mount(serverWith({ authServiceProvider: AUTH_OK })), 'GET', PKGS); + const unwired = await drive(mount(serverWith({ authServiceProvider: AUTH_OK })), 'POST', PUBLISH_PATH); const failed = await drive( mount(serverWith({ ...healthy(), objectQLProvider: async () => { throw new Error('datasource unavailable'); } })), - 'GET', PKGS, + 'POST', PUBLISH_PATH, ); // The repair: the answers DIFFER. Before this card both were @@ -578,13 +579,13 @@ describe('[#13255] consequence — the door\'s answer for each fault class', () // the CAPABILITY refusal an authenticated caller gets, not a fault wearing // the same number: an unwired embedder still RESOLVES an identity. const ctx = await serverWith({ authServiceProvider: AUTH_OK }) - .resolvePackageRouteExecutionContext({ params: {}, headers: {}, method: 'GET', path: PKGS }); + .resolvePackageRouteExecutionContext({ params: {}, headers: {}, method: 'POST', path: PUBLISH_PATH }); expect(ctx?.userId).toBe('u_admin'); expect(ctx?.systemPermissions ?? []).toEqual([]); // ⭐ CONTROL that the harness can still produce the SERVED answer, so the // two refusals above are read as caused by their faults. - expect((await drive(mount(serverWith(healthy())), 'GET', PKGS)).status).toBe(200); + expect((await drive(mount(serverWith(healthy())), 'POST', PUBLISH_PATH)).status).toBe(200); }); it('⭐ [#13476] a provider that RESOLVES `undefined` is "no engine", not a fault', async () => { @@ -595,7 +596,7 @@ describe('[#13255] consequence — the door\'s answer for each fault class', () // fails if `wiredEngineOrLoud` is ever "simplified" into treating any falsy // resolution as a failure. const captured = await drive( - mount(serverWith({ ...healthy(), objectQLProvider: async () => undefined })), 'GET', PKGS); + mount(serverWith({ ...healthy(), objectQLProvider: async () => undefined })), 'POST', PUBLISH_PATH); expect(captured.status).toBe(FORBID.status); expect(captured.body?.error?.code).toBe(FORBID.code); }); @@ -607,12 +608,12 @@ describe('[#13255] consequence — the door\'s answer for each fault class', () // path; it is the instrument that tells "evaluated and holds nothing" // apart from "never evaluated". const lost = CLASSES.find((c) => c.id === 'AUTH_SERVICE_DOWN')!; - const captured = await drive(mount(serverWith(lost.faulted())), 'GET', PKGS, { method: 'OPTIONS' }); + const captured = await drive(mount(serverWith(lost.faulted())), 'POST', PUBLISH_PATH, { method: 'OPTIONS' }); expect(captured.status).toBe(403); expect(captured.body?.error?.code).toBe('FORBIDDEN'); // CONTROL: past the SAME clause, a healthy stack is served. - const control = await drive(mount(serverWith(healthy())), 'GET', PKGS, { method: 'OPTIONS' }); + const control = await drive(mount(serverWith(healthy())), 'POST', PUBLISH_PATH, { method: 'OPTIONS' }); expect(control.status).toBe(200); }); }); @@ -630,7 +631,7 @@ describe('[#13255] the private resolver FULFILS on every production fault class' it.each(CLASSES)('$id — `resolveExecCtx` settles the way its cohort declares', async (klass) => { const rest = serverWith(klass.faulted()); - const req: Record = { params: {}, headers: {}, method: 'GET', path: PKGS, ...(klass.req ?? {}) }; + const req: Record = { params: {}, headers: {}, method: 'POST', path: PUBLISH_PATH, ...(klass.req ?? {}) }; // The PRIVATE resolver, read BEFORE the wrapper's `.catch` can act — so // this reads the supplier, not the net over it. const inner = (rest as any).resolveExecCtx(req.params?.environmentId, req); @@ -649,7 +650,7 @@ describe('[#13255] the private resolver FULFILS on every production fault class' // supplier rather than of the wrapper. const rest = serverWith(healthy()); (rest as any).computeExecCtx = async () => { throw new Error('injected inner rejection'); }; - const req = { params: {}, headers: {}, method: 'GET', path: PKGS }; + const req = { params: {}, headers: {}, method: 'POST', path: PUBLISH_PATH }; expect(await settle((rest as any).resolveExecCtx(undefined, req))).toBe('rejected'); expect(await settle(rest.resolvePackageRouteExecutionContext({ ...req }))).toBe('fulfilled'); expect(await rest.resolvePackageRouteExecutionContext({ ...req })).toBeUndefined(); @@ -665,9 +666,9 @@ describe('[#13255] a server-side fault is indistinguishable from the denial it i it('CONTEXT LOST: an auth-service outage answers exactly what a genuine anonymous caller answers', async () => { const faulted = await drive( mount(serverWith({ ...healthy(), authServiceProvider: async () => { throw new Error('auth service unavailable'); } })), - 'GET', PKGS, + 'POST', PUBLISH_PATH, ); - const anonymous = await drive(mount(serverWith({})), 'GET', PKGS); + const anonymous = await drive(mount(serverWith({})), 'POST', PUBLISH_PATH); expect(faulted.status).toBe(ANONYMOUS_DENY_STATUS); expect(JSON.stringify(faulted)).toBe(JSON.stringify(anonymous)); }); @@ -686,23 +687,26 @@ describe('[#13255] a server-side fault is indistinguishable from the denial it i // verbatim 「第一批其余同意」: 权限库不可达时不再解析为「已认证零能力」, // 而是响亮拒绝(与真实能力拒绝的 403 可区分). const faulted = await drive( - mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'GET', PKGS, + mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'POST', PUBLISH_PATH, ); const genuinelyEmpty = await drive( - mount(serverWith({ authServiceProvider: AUTH_OK, objectQLProvider: async () => qlEmpty() })), 'GET', PKGS, + mount(serverWith({ authServiceProvider: AUTH_OK, objectQLProvider: async () => qlEmpty() })), 'POST', PUBLISH_PATH, ); // The outage is answered as an outage — and says so, in words that cannot // be read as a permission verdict. expect(faulted.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); expect(faulted.body?.error?.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); - expect(faulted.body?.error?.message).not.toContain('studio.access'); + expect(faulted.body?.error?.message).not.toContain('manage_metadata'); // ⚠️ The other half of the ruling, and the half a one-sided fix would // break: a GENUINE capability denial is untouched. Making outages loud is // only correct if real denials still read as denials. expect(genuinelyEmpty.status).toBe(403); - expect(genuinelyEmpty.body?.error?.message).toContain('studio.access'); + // [#14503] The one route left is the write cohort's: its denial names + // `manage_metadata` (the read cohort's `studio.access` wording went with + // the read routes to the dispatcher domain). + expect(genuinelyEmpty.body?.error?.message).toContain('manage_metadata'); // The disguise is gone, stated on the same comparison that pinned it. expect(JSON.stringify(faulted)).not.toBe(JSON.stringify(genuinelyEmpty)); @@ -710,18 +714,18 @@ describe('[#13255] a server-side fault is indistinguishable from the denial it i it('CONTROL: the same comparison SEPARATES two answers that differ', async () => { const [refused, served] = await Promise.all([ - drive(mount(serverWith({})), 'GET', PKGS), - drive(mount(serverWith(healthy())), 'GET', PKGS), + drive(mount(serverWith({})), 'POST', PUBLISH_PATH), + drive(mount(serverWith(healthy())), 'POST', PUBLISH_PATH), ]); expect(JSON.stringify(refused)).not.toBe(JSON.stringify(served)); }); it('and the two DISGUISES are not each other — the door distinguishes lost-context from lost-grants', async () => { const lost = await drive( - mount(serverWith({ ...healthy(), authServiceProvider: async () => { throw new Error('down'); } })), 'GET', PKGS, + mount(serverWith({ ...healthy(), authServiceProvider: async () => { throw new Error('down'); } })), 'POST', PUBLISH_PATH, ); const grants = await drive( - mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'GET', PKGS, + mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'POST', PUBLISH_PATH, ); expect(lost.status).not.toBe(grants.status); }); @@ -739,9 +743,6 @@ describe('[#13255] no degraded class is ever served as anonymous ACCESS or as a const routes = mount(serverWith(klass.faulted())); const extra = klass.req ?? {}; for (const call of [ - () => drive(routes, 'GET', PKGS, extra), - () => drive(routes, 'GET', `${PKGS}/:id`, { ...extra, params: { ...(extra.params ?? {}), id: 'com.acme.crm' } }), - () => drive(routes, 'DELETE', `${PKGS}/:id`, { ...extra, params: { ...(extra.params ?? {}), id: 'com.acme.crm' } }), () => drive(routes, 'POST', `${PKGS}/publish`, { ...extra, body: { manifest: { id: 'com.acme.crm', version: '1.0.0' } } }), ]) { const captured = await call(); @@ -786,9 +787,9 @@ describe('[#13280] at a post-identity provider seam, sync-throw and rejection AG /** The same seam, failed both ways; the door's answer to each. */ const bothShapes = async (seam: 'settingsServiceProvider' | 'objectQLProvider' | 'authServiceProvider') => { const rejecting = await drive( - mount(serverWith({ ...healthy(), [seam]: async () => { throw new Error('seam unavailable'); } })), 'GET', PKGS); + mount(serverWith({ ...healthy(), [seam]: async () => { throw new Error('seam unavailable'); } })), 'POST', PUBLISH_PATH); const syncThrowing = await drive( - mount(serverWith({ ...healthy(), [seam]: (() => { throw new Error('seam unavailable'); }) as any })), 'GET', PKGS); + mount(serverWith({ ...healthy(), [seam]: (() => { throw new Error('seam unavailable'); }) as any })), 'POST', PUBLISH_PATH); return { rejecting, syncThrowing }; }; @@ -869,7 +870,7 @@ describe('[#13280] at a post-identity provider seam, sync-throw and rejection AG // throw (`qlDown`), so the engine seam RESOLVES here and the error can only // have come from `tryFind`. The two raise sites are driven apart by the // section-3 pin, which fails the ENGINE instead of the reads. - const captured = await drive(mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'GET', PKGS); + const captured = await drive(mount(serverWith({ ...healthy(), objectQLProvider: async () => qlDown() })), 'POST', PUBLISH_PATH); expect(captured.status).toBe(AUTHZ_STORE_UNAVAILABLE_STATUS); expect(captured.body?.error?.code).toBe(AUTHZ_STORE_UNAVAILABLE_CODE); }); diff --git a/packages/rest/src/package-door-execctx-fault-reading.test.ts b/packages/rest/src/package-door-execctx-fault-reading.test.ts index d52557e070..1c213cbd76 100644 --- a/packages/rest/src/package-door-execctx-fault-reading.test.ts +++ b/packages/rest/src/package-door-execctx-fault-reading.test.ts @@ -100,13 +100,19 @@ function mount(options: Record = {}): Map listen: async () => {}, close: async () => {}, } as any; - registerPackageRoutes(server, () => ({ list: async () => [] }) as any, '/api/v1', { + // [#14503] The instrument is `POST /packages/publish` — the one route the + // registrar mounts now; the read route this file used to drive is the + // dispatcher domain's alone. Same resolver, same gate, same seam. + registerPackageRoutes(server, () => ({ publish: async () => ({ success: true }) }) as any, '/api/v1', { resolveExecutionContext: CLEARS_THE_GATE, ...options, } as any); return routes; } +const PUBLISH_PATH = `${PKGS}/publish`; +const PUBLISH_BODY = { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }; + async function drive( routes: Map, method: string, @@ -129,13 +135,13 @@ async function drive( return captured; } -/** `GET /packages` under one resolver wiring. `undefined` ⇒ no resolver wired. */ -const listUnder = (resolveExecutionContext: unknown, req: Record = {}) => +/** `POST /packages/publish` under one resolver wiring. `undefined` ⇒ no resolver wired. */ +const publishUnder = (resolveExecutionContext: unknown, req: Record = {}) => drive( mount(resolveExecutionContext === undefined ? { resolveExecutionContext: undefined } : { resolveExecutionContext }), - 'GET', - PKGS, - req, + 'POST', + PUBLISH_PATH, + { body: PUBLISH_BODY, ...req }, ); /** The three ways this door can end up holding `undefined`. */ @@ -149,13 +155,13 @@ const RESOLVES_UNDEFINED = async () => undefined; describe('[#12537] controls — the instrument produces a one before any zero is read', () => { it('CONTROL (allow is observable): a fully capable context is served 200', async () => { - const captured = await listUnder(CLEARS_THE_GATE); + const captured = await publishUnder(CLEARS_THE_GATE); expect(captured.status).toBe(200); expect(captured.body?.success).toBe(true); }); it('CONTROL (the anonymous clause is observable): a named subject is NOT 401', async () => { - const captured = await listUnder(async () => ({ userId: 'u_named', systemPermissions: [] })); + const captured = await publishUnder(async () => ({ userId: 'u_named', systemPermissions: [] })); expect(captured.status).not.toBe(ANONYMOUS_DENY_STATUS); expect(captured.status).toBe(403); expect(captured.body?.error?.code).toBe('FORBIDDEN'); @@ -163,7 +169,7 @@ describe('[#12537] controls — the instrument produces a one before any zero is it('CONTROL (the resolver really runs): the door calls it exactly once per request', async () => { const resolver = vi.fn(REJECTS); - await listUnder(resolver); + await publishUnder(resolver); expect(resolver.mock.calls.length).toBe(1); }); @@ -178,14 +184,14 @@ describe('[#12537] controls — the instrument produces a one before any zero is describe('[#12537] a swallowed resolution does not fall through to a system subject', () => { it('a rejecting resolver is REFUSED, not served', async () => { - const captured = await listUnder(REJECTS); + const captured = await publishUnder(REJECTS); expect(captured.status).toBe(ANONYMOUS_DENY_STATUS); expect(captured.body?.success).toBe(false); expect(captured.body?.error?.code).toBe(ANONYMOUS_DENY_CODE); }); it('CONTROL: a real system subject IS served — so "refused" above is a decision, not an artefact', async () => { - const captured = await listUnder(async () => ({ isSystem: true })); + const captured = await publishUnder(async () => ({ isSystem: true })); expect(captured.status).toBe(200); expect(captured.body?.success).toBe(true); }); @@ -197,36 +203,38 @@ describe('[#12537] a swallowed resolution does not fall through to a system subj describe('[#12537] a swallowed resolution does not bypass the gate', () => { it('the service is never reached when the resolver rejects', async () => { - const list = vi.fn(async () => []); + const publish = vi.fn(async () => ({ success: true })); const routes = new Map(); const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: () => {}, put: () => {}, delete: () => {}, patch: () => {}, + get: () => {}, + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, + put: () => {}, delete: () => {}, patch: () => {}, use: () => {}, listen: async () => {}, close: async () => {}, } as any; - registerPackageRoutes(server, () => ({ list }) as any, '/api/v1', { + registerPackageRoutes(server, () => ({ publish }) as any, '/api/v1', { resolveExecutionContext: REJECTS, } as any); - const captured = await drive(routes, 'GET', PKGS); + const captured = await drive(routes, 'POST', PUBLISH_PATH, { body: PUBLISH_BODY }); expect(captured.status).toBe(ANONYMOUS_DENY_STATUS); - // ⚠️ ZERO. Its control is the next assertion, on the SAME `list` spy shape. - expect(list.mock.calls.length).toBe(0); + // ⚠️ ZERO. Its control is the next assertion, on the SAME `publish` spy shape. + expect(publish.mock.calls.length).toBe(0); }); it('CONTROL: the same spy DOES record a call when the gate is cleared', async () => { - const list = vi.fn(async () => []); + const publish = vi.fn(async () => ({ success: true })); const routes = new Map(); const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: () => {}, put: () => {}, delete: () => {}, patch: () => {}, + get: () => {}, + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, + put: () => {}, delete: () => {}, patch: () => {}, use: () => {}, listen: async () => {}, close: async () => {}, } as any; - registerPackageRoutes(server, () => ({ list }) as any, '/api/v1', { + registerPackageRoutes(server, () => ({ publish }) as any, '/api/v1', { resolveExecutionContext: CLEARS_THE_GATE, } as any); - const captured = await drive(routes, 'GET', PKGS); + const captured = await drive(routes, 'POST', PUBLISH_PATH, { body: PUBLISH_BODY }); expect(captured.status).toBe(200); - expect(list.mock.calls.length).toBe(1); + expect(publish.mock.calls.length).toBe(1); }); }); @@ -235,32 +243,32 @@ describe('[#12537] a swallowed resolution does not bypass the gate', () => { // subject holding the EMPTY set. // // ⚠️ DISCLOSURE, so no reader mistakes this for a wire path: the packages -// registrar mounts exactly four routes (POST publish, GET list, GET by id, -// DELETE by id) and NO `OPTIONS` route, so a real preflight never reaches -// these handlers. `method: 'OPTIONS'` is used here as the one INPUT that -// makes the shared `shouldDenyAnonymous` yield without authenticating — -// i.e. as an instrument for isolating the capability clause from the -// anonymous clause, which otherwise short-circuits ahead of it. That -// isolation is the only way to tell "evaluated and holds nothing" apart -// from "never evaluated": on a plain GET both readings answer 401. +// registrar mounts exactly one route (POST publish, since #14503) and NO +// `OPTIONS` route, so a real preflight never reaches this handler. +// `method: 'OPTIONS'` is used here as the one INPUT that makes the shared +// `shouldDenyAnonymous` yield without authenticating — i.e. as an +// instrument for isolating the capability clause from the anonymous +// clause, which otherwise short-circuits ahead of it. That isolation is the +// only way to tell "evaluated and holds nothing" apart from "never +// evaluated": on a plain POST both readings answer 401. // --------------------------------------------------------------------------- describe('[#12537] the capability clause reads `undefined` as "holds nothing"', () => { it('past the anonymous clause, a swallowed resolution is 403 FORBIDDEN', async () => { - const captured = await listUnder(REJECTS, { method: 'OPTIONS' }); + const captured = await publishUnder(REJECTS, { method: 'OPTIONS' }); expect(captured.status).toBe(403); expect(captured.body?.error?.code).toBe('FORBIDDEN'); - expect(captured.body?.error?.message).toContain('studio.access'); + expect(captured.body?.error?.message).toContain('manage_metadata'); }); it('CONTROL: past the same clause, a CAPABLE context is served 200', async () => { - const captured = await listUnder(CLEARS_THE_GATE, { method: 'OPTIONS' }); + const captured = await publishUnder(CLEARS_THE_GATE, { method: 'OPTIONS' }); expect(captured.status).toBe(200); expect(captured.body?.success).toBe(true); }); it('CONTROL: past the same clause, an explicit EMPTY capability set is the same 403', async () => { - const captured = await listUnder( + const captured = await publishUnder( async () => ({ userId: 'u_named', systemPermissions: [] }), { method: 'OPTIONS' }, ); @@ -278,9 +286,9 @@ describe('[#12537] the capability clause reads `undefined` as "holds nothing"', describe('[#12537] a resolver FAULT is indistinguishable from anonymity and from no resolver', () => { it('rejecting resolver, resolver returning undefined, and no resolver agree byte for byte', async () => { const [faulted, anonymous, unwired] = await Promise.all([ - listUnder(REJECTS), - listUnder(RESOLVES_UNDEFINED), - listUnder(undefined), + publishUnder(REJECTS), + publishUnder(RESOLVES_UNDEFINED), + publishUnder(undefined), ]); expect(faulted.status).toBe(ANONYMOUS_DENY_STATUS); expect(JSON.stringify(faulted)).toBe(JSON.stringify(anonymous)); @@ -288,18 +296,20 @@ describe('[#12537] a resolver FAULT is indistinguishable from anonymity and from }); it('CONTROL: the same comparison SEPARATES two answers that differ', async () => { - const [faulted, capable] = await Promise.all([listUnder(REJECTS), listUnder(CLEARS_THE_GATE)]); + const [faulted, capable] = await Promise.all([publishUnder(REJECTS), publishUnder(CLEARS_THE_GATE)]); expect(JSON.stringify(faulted)).not.toBe(JSON.stringify(capable)); }); - it('every state-changing route reads the fault the same way', async () => { + it('the state-changing route reads the fault the same way with a body it would otherwise act on', async () => { + // [#14503] Used to compare `DELETE /:id` against publish; the delete route + // is the dispatcher domain's alone now, so the one state-changing route + // left is driven with a manifest it would publish if the gate let it. const routes = mount({ resolveExecutionContext: REJECTS }); - const del = await drive(routes, 'DELETE', `${PKGS}/:id`, { params: { id: 'com.acme.crm' } }); const pub = await drive(routes, 'POST', `${PKGS}/publish`, { - body: { manifest: { id: 'com.acme.crm', version: '1.0.0' } }, + body: { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }, }); - expect(del.status).toBe(ANONYMOUS_DENY_STATUS); expect(pub.status).toBe(ANONYMOUS_DENY_STATUS); + expect(pub.body?.error?.code).toBe(ANONYMOUS_DENY_CODE); }); }); @@ -376,7 +386,7 @@ describe('[#12537] a SYNC throw is forwarded from the producer, not decided by t const throwsSync = (error: unknown) => () => { throw error; }; it('a coded producer error keeps ITS status — the thread\'s 403 is this, not a gate decision', async () => { - const captured = await listUnder( + const captured = await publishUnder( throwsSync(Object.assign(new Error('nope'), { code: 'PERMISSION_DENIED', status: 403 })), ); expect(captured.status).toBe(403); @@ -384,13 +394,13 @@ describe('[#12537] a SYNC throw is forwarded from the producer, not decided by t }); it('the SAME seam, thrown a plain Error, answers 500 — so the status tracks the error', async () => { - const captured = await listUnder(throwsSync(new Error('nope'))); + const captured = await publishUnder(throwsSync(new Error('nope'))); expect(captured.status).toBe(500); expect(captured.body?.error?.code).not.toBe('PERMISSION_DENIED'); }); it('and neither of those is the swallowed case: a REJECTION is still the 401 floor', async () => { - const captured = await listUnder( + const captured = await publishUnder( async () => { throw Object.assign(new Error('nope'), { code: 'PERMISSION_DENIED', status: 403 }); }, ); expect(captured.status).toBe(ANONYMOUS_DENY_STATUS); diff --git a/packages/rest/src/package-door-producer-key-carry.test.ts b/packages/rest/src/package-door-producer-key-carry.test.ts deleted file mode 100644 index 4a5565ff98..0000000000 --- a/packages/rest/src/package-door-producer-key-carry.test.ts +++ /dev/null @@ -1,463 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * GATE — the REST `/packages` door's field allowlist must not silently drop a - * key the PRODUCER stamps. - * - * ## The defect this exists to catch - * - * `REGISTRY_PACKAGE_RESPONSE_FIELDS` (`package-routes.ts`) is a hand-written - * allowlist, and that was a deliberate trade: a newly declared or - * producer-stamped field becomes an explicit decision at this door, and drift - * shows up as a **missing field** rather than as the `500 Converting circular - * structure to JSON` the projection replaced. - * - * The other half of that trade is this file. Within one day of the allowlist - * landing, the protocol started stamping an ADR-0070 D2 `writable` verdict on - * every `getMetaItems({ type: 'package' })` row. The allowlist did not list it, - * so the door would have answered **200 with the verdict simply absent** — no - * 500, no red, and nothing on the wire a consumer could tell apart from "this - * package has no verdict". It was caught by someone happening to read a sibling - * pin. That is not a mechanism. - * - * ⚠️ `writable` itself is now pinned by name in - * `package-list-writable-carry.test.ts`, so the *known* field is covered. What - * was missing — and is what this file supplies — is the GENERAL case: **nothing - * generalised to the NEXT stamped key.** The next ADR-0070-style verdict lands - * with exactly the same silence. - * - * ## The invariant, and why it is derived rather than listed - * - * served key set ⊇ producer key set − DELIBERATELY_NOT_SERVED - * - * Both sides are MEASURED by running real code in this test: - * - * - the producer side is the real `ObjectStackProtocolImplementation` - * (`getMetaItems({ type: 'package' })`) over a real `SchemaRegistry` — the - * same path production reads, including the `writable` stamp and the - * `decorateMetadataItem` graft; - * - the served side is this door's real `GET /api/v1/packages` output. - * - * ⛔ Neither side is a hand-written key list. A fixture that hand-listed the - * producer's keys would be a THIRD copy of the same truth and would drift - * alongside the two it is meant to compare. The only hand-kept artifact here is - * {@link DELIBERATELY_NOT_SERVED} — an explicit, annotated exclusion register, - * which the card requires to be exactly that: a decision that must be written - * down, never an omission that accumulates in silence. - * - * ⛔ The allowlist is NOT derived from `packages/spec`. That was weighed and - * rejected on the originating card (it makes the published surface a side - * effect of a schema edit, and adds a spec import edge to `@objectstack/rest`). - * This file is the detector that makes the hand-list honest — not a re-opening - * of that decision. - * - * ## Why the invariant is `⊇` and not `=` - * - * This door must KEEP dropping an undeclared member that leaks onto a registry - * item — that is the whole point of projecting rather than spreading, and a - * set-equality gate would force such a member back onto the wire and re-open - * the `500`. So the gate is one-directional, and the fixture installs its - * packages through the real `SchemaRegistry.installPackage`: over a clean - * install the producer's key set IS "declared record fields + producer stamps", - * which is exactly the set that must survive. A future producer key that - * genuinely should not be published therefore arrives here as a red, and is - * answered by writing it into {@link DELIBERATELY_NOT_SERVED} with a reason — - * an explicit decision at the door, which is what the card asked for. The last - * test in this file pins the drop so the two claims stay visibly compatible. - * - * ## The twin door has a DIFFERENT invariant — do not assume symmetry - * - * The runtime dispatcher twin (`packages/runtime/src/domains/packages.ts`) - * solved the same near-miss the other way: `writable` is NOT an allowlist - * member there, and the door stamps it AFTER the projection instead. Asserting - * "the allowlist contains every stamped key" over there would red on a correct - * door. Its own half of this gate is - * `packages/runtime/src/domains/package-door-producer-key-carry.test.ts`, which - * states the ordering invariant instead. Two pins, one shape — see that file's - * header for why they are not one file. - */ - -import { describe, it, expect } from 'vitest'; -import { SchemaRegistry } from '@objectstack/objectql'; -import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { InstalledPackageSchema } from '@objectstack/spec/kernel'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; - -/** - * Keys the producer puts on a package row that this door deliberately does NOT - * serve. **Explicit and annotated by construction** — an entry added here is a - * published-surface decision someone wrote down, which is the whole difference - * between this register and the silent omission it replaces. - * - * Empty today: every key the real producer stamps on a package row is carried. - * Measured, not assumed — {@link producerKeysOf} reads the real - * `getMetaItems({ type: 'package' })` output, and the assertion below fails - * with the offending key names when that stops being true. - * - * ⛔ Adding a key here to make a red test green is the defect one level up. The - * question an entry must answer is "why must this door withhold it?", and the - * answer belongs in the comment beside it. - */ -const DELIBERATELY_NOT_SERVED: readonly string[] = []; - -/** The engine's own `actionActivation -> store -> engine` cycle, reproduced. */ -function cyclicEngine(): Record { - const engine: Record = { name: '_ObjectQL' }; - const store: Record = { name: 'ObjectStoreActionActivationStore', engine }; - engine.actionActivation = { name: 'ActionActivationProjection', store }; - return engine; -} - -/** A host-constructed connector plugin that takes the engine on init. */ -class FakeConnectorPlugin { - name = 'connector-rest'; - engine: unknown; - init(engine: unknown) { this.engine = engine; } -} - -/** Booted app package, explicit `scope: 'project'` — the producer says read-only. */ -const CODE_PROJECT = 'com.example.showcase'; -/** Platform-delivered plugin package. */ -const SYSTEM_SCOPED = 'com.objectstack.setup'; -/** Studio-created database base: installed, never booted, scope-less. */ -const DB_BASE = 'com.acme.mybase'; - - -/** - * The keys the installed-package RECORD declares, read from the record schema - * ITSELF rather than typed out here. - * - * ⚠️ Read this before mistaking it for the design the originating card - * rejected. What was weighed and rejected there is deriving the **production - * allowlist** from `packages/spec` — that would publish a newly declared field - * automatically, making the served surface a side effect of a schema edit. The - * allowlist stays hand-written and is untouched by this file. What is derived - * here is the TEST's EXPECTATION, which is the card's own wording for the - * detector: "the projected key set ⊇ the producer's stamped/**declared** key - * set minus an explicit, annotated exclusion list". Deriving the expectation is - * what turns a newly declared field into a red test — an explicit decision at - * the door — instead of a silent omission. It adds no import edge either: - * `@objectstack/spec` is already a runtime dependency of this package. - * - * Measured, and the reason this exists at all: `installPackage` writes only the - * fields an install can know (`manifest`, `status`, `enabled`, `installedAt`, - * `updatedAt`, `settings`). The five lifecycle fields the record also declares - * — `installedVersion`, `previousVersion`, `statusChangedAt`, `errorMessage`, - * `upgradeHistory`, `registeredNamespaces` — are simply ABSENT from a freshly - * installed record, so a gate that watched only the producer's live output - * could not see them dropped: deleting `installedVersion` from the allowlist - * left this gate GREEN on its first ablation. - */ -const DECLARED_RECORD_KEYS: readonly string[] = Object.keys( - (InstalledPackageSchema as unknown as { shape: Record }).shape, -); - -/** - * Give every DECLARED field a value on this record, so the door can be observed - * either carrying it or dropping it. - * - * Without this the coverage assertion silently shrinks to the handful of fields - * a fresh install happens to write, while reading as though it covered the - * record. The values are deliberately meaningless — this gate compares KEY - * SETS, and what each field must CONTAIN is pinned by that field's own tests. - */ -function seatDeclaredFields(record: Record): void { - for (const k of DECLARED_RECORD_KEYS) { - if (record[k] === undefined) record[k] = `__seated__${k}`; - } -} - -/** - * A registry in the showcase's shape, built through the REAL - * `SchemaRegistry.installPackage` — so the records under test are the records - * production holds, not a literal someone typed next to the assertion. - */ -function realRegistry(): SchemaRegistry { - const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); - (registry as unknown as { logLevel: string }).logLevel = 'silent'; - - const plugin = new FakeConnectorPlugin(); - registry.installPackage({ - id: CODE_PROJECT, - name: 'Showcase', - namespace: 'showcase', - version: '0.3.16', - type: 'app', - scope: 'project', - description: 'Kitchen-sink showcase workspace', - objects: [{ name: 'invoice', fields: { total: { type: 'currency' } } }], - apps: [{ name: 'showcase', label: 'Showcase' }], - plugins: [plugin], - } as never); - // Init AFTER install — the measured ordering: the manifest serialised cleanly - // during boot and only became cyclic once the plugins came up. - plugin.init(cyclicEngine()); - - registry.installPackage({ - id: SYSTEM_SCOPED, name: 'Setup', namespace: 'setup', version: '9.3.0', - type: 'plugin', scope: 'system', - } as never); - - registry.installPackage({ - id: DB_BASE, name: 'My Base', namespace: 'mybase', version: '1.0.0', type: 'app', - } as never); - - // Every declared field carries a value from here on — see - // `seatDeclaredFields` for the measurement that made this necessary. - for (const id of [CODE_PROJECT, SYSTEM_SCOPED, DB_BASE]) { - seatDeclaredFields(registry.getPackage(id) as unknown as Record); - } - - return registry; -} - -/** - * The real producer, over that registry. `manifests` is what `ObjectQL.registerApp` - * records for every package of a loaded artifact — the ADR-0070 D2 predicate - * reads it FIRST, so it is what separates a booted (read-only) package from a - * Studio-created (writable) base and makes the `writable` stamp non-constant. - */ -function realProducer(registry: SchemaRegistry): ObjectStackProtocolImplementation { - const engine: Record = { - registry, - manifests: new Map([ - [CODE_PROJECT, registry.getPackage(CODE_PROJECT)?.manifest], - [SYSTEM_SCOPED, registry.getPackage(SYSTEM_SCOPED)?.manifest], - ]), - // No `sys_metadata` overlay in this fixture: the subject is the registry - // half's key set, and an overlay row would only add rows, not keys. - // - // `find` is the ONLY engine verb `getMetaItems` reaches (the rest of the - // read goes through `engine.registry`, which is real here). Deliberately no - // `findOne` double: it belongs to the single-item read this file never - // drives, and a fake looser than `ObjectQL.findOne` is how a dead REST route - // once shipped with its suite green — so the honest fixture omits the verb - // rather than stubbing it. `check:engine-double-contract` enforces that. - find: async () => [], - }; - return new ObjectStackProtocolImplementation(engine as never, () => new Map()); -} - -type ProducerRow = Record & { manifest?: { id?: unknown } }; - -/** - * The keys of `row` that a RESPONSE can actually carry. - * - * `Object.keys` alone is the wrong instrument here and the difference is - * measured, not theoretical: `SchemaRegistry.installPackage` seats the optional - * record fields as own properties holding `undefined`, so a package installed - * without settings still answers `'settings' in record === true`. Both doors - * omit undefined-valued fields on purpose ("the bytes are unchanged for every - * entry that already served fine"), and `JSON.stringify` would drop them - * anyway — so a gate counting them would red on every package for a key no - * consumer could ever have observed, and the first repair anyone reached for - * would be to widen the exclusion register until the real signal was buried. - * - * What this gate is about is a key that HAS a value and does not reach the - * wire. That is the near-miss (`writable: false` is a value), and it is what - * this filter keeps in view. - */ -const definedKeys = (row: Record): Set => - new Set(Object.keys(row).filter((k) => row[k] !== undefined)); - -/** The row id, keyed the way both the producer and the door key it. */ -function rowId(row: ProducerRow): string | undefined { - const fromManifest = row?.manifest?.id; - if (typeof fromManifest === 'string') return fromManifest; - return typeof row.id === 'string' ? row.id : undefined; -} - -/** MEASURE the producer's key set — never a literal. */ -async function producerKeysOf( - protocol: ObjectStackProtocolImplementation, -): Promise>> { - const res = await protocol.getMetaItems({ type: 'package' }); - const out = new Map>(); - for (const item of (res.items ?? []) as ProducerRow[]) { - const id = rowId(item); - if (id) out.set(id, definedKeys(item)); - } - return out; -} - -interface Captured { status: number; body: any } - -/** A durable half that finds nothing, so only the registry half is exercised. */ -const EMPTY_DB = { list: async () => [], get: async () => null }; - -function mount(protocol: ObjectStackProtocolImplementation) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as never; - // The authorization gate (#7033 / #7023) is not this file's subject. - registerPackageRoutes(server, (() => EMPTY_DB) as never, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - // The REAL producer, bound as the door's protocol seam. - protocol: { getMetaItems: (req: never) => protocol.getMetaItems(req) }, - } as never); - return routes; -} - -async function drive( - routes: Map, - method: string, - path: string, - req: Record = {}, -): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(data: any) { captured.body = data; }, - send() {}, - status(code: number) { captured.status = code; return res; }, - header() { return res; }, - }; - await handler( - { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as never, - res, - ); - return captured; -} - -/** - * THE DETECTOR. Shared in shape with the runtime twin: the keys a door drops - * are `producer − served − excluded`, and a non-empty answer is the defect. - * - * Returns the dropped keys rather than asserting, so the caller can name the - * row in the failure message — a bare "expected [] to equal ['writable']" does - * not say which package or which door. - */ -function droppedKeys( - producer: ReadonlySet, - served: ReadonlySet, - excluded: readonly string[], -): string[] { - const exempt = new Set(excluded); - return [...producer].filter((k) => !served.has(k) && !exempt.has(k)).sort(); -} - -describe('GATE: REST GET /packages carries every key the producer stamps', () => { - it('control: the producer really does stamp keys the RECORD does not declare', async () => { - // ANTI-VACUITY. If the producer stamped nothing beyond the stored record, - // the coverage assertion below could pass over an allowlist that had never - // been asked a hard question. `writable` (ADR-0070 D2) is the stamp the - // near-miss was about, so its presence is what makes this gate load-bearing - // — and its absence would mean the producer changed under us, which is a - // decision, not a green. - const registry = realRegistry(); - const perRow = await producerKeysOf(realProducer(registry)); - - expect([...perRow.keys()].sort()).toEqual([DB_BASE, CODE_PROJECT, SYSTEM_SCOPED].sort()); - - // The schema read must have WORKED. `InstalledPackageSchema` is a lazy - // proxy; a read that silently produced `{}` would make DECLARED_RECORD_KEYS - // empty and every assertion below vacuous, in a suite that stayed green. - expect(DECLARED_RECORD_KEYS.length).toBeGreaterThanOrEqual(8); - expect(DECLARED_RECORD_KEYS).toContain('installedVersion'); - - // …and every declared slot must be OBSERVABLE on the record, or the - // coverage assertion silently shrinks to whatever a fresh install wrote. - // Measured before `seatDeclaredFields` read the schema: 6 of the 12 - // declared fields were absent from the record, and deleting one of them - // from the allowlist left this gate green. - const record = registry.getPackage(CODE_PROJECT) as unknown as Record; - const unobservable = DECLARED_RECORD_KEYS.filter((k) => record[k] === undefined); - expect(unobservable, 'declared fields the fixture leaves unobservable').toEqual([]); - - const recordKeys = definedKeys(record); - const stamped = [...perRow.get(CODE_PROJECT)!].filter((k) => !recordKeys.has(k)); - expect(stamped).toContain('writable'); - }); - - it('every producer key survives to the wire, for every package', async () => { - // THE GATE. Remove a stamped key from `REGISTRY_PACKAGE_RESPONSE_FIELDS` — - // or land a new producer stamp without adding it — and this reds with the - // key's own name, instead of shipping a 200 with the field absent. - const registry = realRegistry(); - const protocol = realProducer(registry); - const perRow = await producerKeysOf(protocol); - - const res = await drive(mount(protocol), 'GET', PKGS); - expect(res.status).toBe(200); - - const served: ProducerRow[] = res.body?.data?.packages ?? []; - expect(served.length).toBe(perRow.size); - - const routes = mount(protocol); - const report: string[] = []; - for (const [id, producerKeys] of perRow) { - const row = served.find((p) => rowId(p) === id); - expect(row, `package ${id} vanished from the response entirely`).toBeDefined(); - const dropped = droppedKeys(producerKeys, definedKeys(row as ProducerRow), DELIBERATELY_NOT_SERVED); - if (dropped.length) report.push(`list ${id}: ${dropped.join(', ')}`); - - // The DETAIL door runs the same producer through the same projection, and - // is the other place a dropped key would ship as a 200. - const one = await drive(routes, 'GET', `${PKGS}/:id`, { params: { id } }); - expect(one.status, `GET ${PKGS}/${id}`).toBe(200); - const detail = one.body?.data?.package as ProducerRow | undefined; - expect(detail, `package ${id} vanished from the detail response`).toBeDefined(); - const detailDropped = droppedKeys(producerKeys, definedKeys(detail as ProducerRow), DELIBERATELY_NOT_SERVED); - if (detailDropped.length) report.push(`detail ${id}: ${detailDropped.join(', ')}`); - } - - // The failure text names the door, the package and the key, because the - // reader of a red here is someone who just added a producer stamp and needs - // to be told which allowlist to decide about. - expect( - report, - 'REST GET /packages dropped producer-stamped key(s). Either add them to ' - + '`REGISTRY_PACKAGE_RESPONSE_FIELDS` in package-routes.ts, or record the ' - + 'withholding in `DELIBERATELY_NOT_SERVED` in this file with the reason.', - ).toEqual([]); - }); - - it('control: the detector reports a dropped key rather than passing vacuously', async () => { - // Proves the assertion above can FAIL, without mutating source. The door is - // real and so is the drop: an undeclared key on the producer's row is - // exactly what the allowlist deletes, and the detector must say so. - const registry = realRegistry(); - const protocol = realProducer(registry); - const real = await producerKeysOf(protocol); - - const res = await drive(mount(protocol), 'GET', PKGS); - const row = (res.body?.data?.packages ?? []).find((p: ProducerRow) => rowId(p) === CODE_PROJECT); - const servedKeys = definedKeys(row as ProducerRow); - - // A hypothetical next verdict, stamped by the producer and not yet decided - // about at this door. - const withNextStamp = new Set([...real.get(CODE_PROJECT)!, 'nextVerdict']); - expect(droppedKeys(withNextStamp, servedKeys, DELIBERATELY_NOT_SERVED)).toEqual(['nextVerdict']); - // …and the exclusion register is the one way to make that green again. - expect(droppedKeys(withNextStamp, servedKeys, ['nextVerdict'])).toEqual([]); - }); - - it('the projection still drops an undeclared LIVE member — the gate does not undo it', async () => { - // The coverage assertion is one-directional (⊇), on purpose: this door must - // keep degrading an unserializable member to a missing field. A gate written - // as set EQUALITY would have forced that member back onto the wire and - // re-opened the 500. - const registry = realRegistry(); - (registry.getPackage(CODE_PROJECT) as Record).liveEngineHandle = cyclicEngine(); - - const res = await drive(mount(realProducer(registry)), 'GET', PKGS); - expect(res.status).toBe(200); - expect(() => JSON.stringify(res.body)).not.toThrow(); - const served: ProducerRow[] = res.body?.data?.packages ?? []; - expect(served.some((p) => 'liveEngineHandle' in p)).toBe(false); - }); -}); diff --git a/packages/rest/src/package-door-user-message.test.ts b/packages/rest/src/package-door-user-message.test.ts index 7a3c118371..921037bb46 100644 --- a/packages/rest/src/package-door-user-message.test.ts +++ b/packages/rest/src/package-door-user-message.test.ts @@ -182,7 +182,7 @@ function thrown(message: string, carried: Record): Error { /** * One catch site, plus the seam that drives a throw INTO it and a witness that - * the throw really travelled that way. Same four seams as + * the throw really travelled that way. Same two seams as * `package-door-declared-code.test.ts` and * `package-routes-coded-error-mapping.test.ts`, for the same reason: a case * that silently never reached the seam would otherwise "pass" on a body it got @@ -208,43 +208,18 @@ const SITES: Site[] = [ }, }, { - name: 'GET /packages — the capability gate resolver throws', + name: 'POST /packages/publish — the capability gate resolver throws', run: async (error: unknown) => { const resolveExecutionContext = vi.fn(() => { throw error; }); const captured = await drive( - mount({ list: async () => [] }, { resolveExecutionContext }), - 'GET', - PKGS, + mount({ publish: async () => ({ success: true }) }, { resolveExecutionContext }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); return { captured, reached: () => resolveExecutionContext.mock.calls.length === 1 }; }, }, - { - name: 'GET /packages/:id — packageService.get throws', - run: async (error: unknown) => { - const get = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ get }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => get.mock.calls.length === 1 }; - }, - }, - { - name: 'DELETE /packages/:id — packageService.delete throws', - run: async (error: unknown) => { - const del = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ delete: del }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => del.mock.calls.length === 1 }; - }, - }, ]; // --------------------------------------------------------------------------- diff --git a/packages/rest/src/package-envelope.conformance.test.ts b/packages/rest/src/package-envelope.conformance.test.ts index deedf22870..167d4c620a 100644 --- a/packages/rest/src/package-envelope.conformance.test.ts +++ b/packages/rest/src/package-envelope.conformance.test.ts @@ -1,7 +1,14 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * Response-envelope conformance for `/api/v1/packages/*` (#3843). + * Response-envelope conformance for the REST package registrar (#3843). + * + * [#14503] The registrar mounts ONE route now — `POST /packages/publish`. The + * `GET /packages`, `GET /packages/:id` and `DELETE /packages/:id` cases this + * file used to carry drove routes that duplicated the dispatcher's `/packages` + * domain and have been removed; that domain is their single implementation + * and `packages/runtime/src/domains/packages-single-door.test.ts` pins its + * envelope. Everything below is the publish route. * * This module was the *partially converted* one, which #3843 called "arguably * worse than untouched": 3 of its 16 bodies carried `success: true` and the rest @@ -47,12 +54,9 @@ interface Captured { body: any; } -/** A `PackageService` stub — only the four methods these routes reach. */ +/** A `PackageService` stub — only the method the publish route reaches. */ type Svc = Partial<{ publish: (arg: any) => Promise; - list: () => Promise; - get: (id: string, version?: string) => Promise; - delete: (id: string, version?: string) => Promise; }>; function mount(svc: Svc, options: any = {}) { @@ -67,9 +71,9 @@ function mount(svc: Svc, options: any = {}) { listen: async () => {}, close: async () => {}, } as any; - // [#7033 / #7023] The package routes now carry an authorization gate. These + // [#7033 / #7023] The package route carries an authorization gate. These // envelope cases are about RESPONSE SHAPE, so the caller is stubbed to clear - // the gate (holding both the read and write capability); a test can override + // the gate (holding the write capability); a test can override // `resolveExecutionContext` to exercise the gate itself. The gate itself is // pinned in the `packages authz` describe at the bottom of this file. registerPackageRoutes(server, () => svc as any, '/api/v1', { @@ -118,55 +122,6 @@ describe('packages envelope (#3843) — success bodies', () => { { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ), }, - { - name: 'GET /packages', - status: 200, - dataKeys: ['packages', 'total'], - run: () => drive( - mount({ list: async () => [{ id: 'com.acme.crm', manifest: MANIFEST }] }), - 'GET', - PKGS, - ), - }, - { - name: 'GET /packages/:id', - status: 200, - dataKeys: ['package'], - run: () => drive( - mount({ get: async () => ({ id: 'com.acme.crm', manifest: MANIFEST }) }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ), - }, - { - name: 'DELETE /packages/:id (version-scoped, durable registry)', - status: 200, - dataKeys: ['message'], - run: () => drive( - mount({ delete: async () => ({ success: true }) }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' }, query: { version: '1.0.0' } }, - ), - }, - { - name: 'DELETE /packages/:id (full uninstall via protocol)', - status: 200, - dataKeys: ['message', 'deletedCount', 'cleanups'], - run: () => drive( - mount({}, { - protocol: { - deletePackage: async () => ({ - success: true, deletedCount: 3, failedCount: 0, failed: [], cleanups: [{ name: 'security', success: true, removed: 2 }], - }), - }, - }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ), - }, ]; for (const c of CASES) { @@ -195,9 +150,9 @@ describe('packages envelope (#3843) — success bodies', () => { it('the two shapes are now one — no payload beside the flag, none without it', async () => { for (const c of CASES) { const { body } = await c.run(); - // The 13 bodies that had no flag. + // The bodies that had no flag. expect(typeof body.success, `${c.name} answers no success flag`).toBe('boolean'); - // The 3 that had one, with the payload spread beside it. + // The ones that had one, with the payload spread beside it. for (const k of c.dataKeys) { expect(body[k], `${c.name} still answers a top-level ${k}`).toBeUndefined(); } @@ -205,20 +160,6 @@ describe('packages envelope (#3843) — success bodies', () => { } }); - it('a registry-only package is still found when the database has none', async () => { - // Guards the fallback arm of GET /:id, which is a separate `sendOk` call. - const { status, body } = await drive( - mount({ get: async () => undefined }, { - protocol: { getMetaItems: async () => ({ items: [{ manifest: MANIFEST }] }) }, - }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - expect(status).toBe(200); - expect(body.success).toBe(true); - expect(body.data.package.source).toBe('registry'); - }); }); describe('packages envelope (#3843) — error bodies', () => { @@ -280,71 +221,28 @@ describe('packages envelope (#3843) — error bodies', () => { ), }, { - name: 'reading a package that does not exist', - status: 404, - code: 'RESOURCE_NOT_FOUND', - run: () => drive( - mount({ get: async () => undefined }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.nope' } }, - ), - }, - { - // Was a bare `{ success: false, failed, cleanups }` — a failure with no - // `error` at all. - name: 'an uninstall that leaves items behind', - status: 400, - code: 'PACKAGE_DELETE_PARTIAL', - run: () => drive( - mount({}, { - protocol: { - deletePackage: async () => ({ - success: false, deletedCount: 1, failedCount: 2, - failed: [{ type: 'object', name: 'invoice', error: 'in use' }], - cleanups: [], - }), - }, - }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ), - }, - { - // And the other one: a bare `{ success: false }`. - // - // [#8275] Re-spelled, not replaced: the fixture and its envelope - // assertion are unchanged, only the STATUS this outcome answers moved. - // A returned failure here means the `DELETE FROM sys_packages` broke — - // a server fault — so it is a 5xx, the sibling of the `publish` case - // above. The code is untouched, and this suite's subject (the declared - // envelope) is unaffected by which status carries it. - name: 'a version-scoped delete whose statement broke — a driver fault, so a 5xx', + name: 'an unexpected throw from the package service', status: 500, - code: 'PACKAGE_DELETE_FAILED', + code: 'INTERNAL_ERROR', run: () => drive( - mount({ delete: async () => ({ success: false }) }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' }, query: { version: '1.0.0' } }, + mount({ publish: async () => { throw new Error('db down'); } }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: {} } }, ), }, { - // [#11063] Was: "NOT `GET /packages` — that route catches a failing - // `list()` in an INNER try and degrades to the registry-only listing, so - // its 500 arm is unreachable that way." That inner catch is gone; both - // read doors now reach this arm. `GET /:id` is kept as this case's - // subject so the case itself is unchanged, and the list door's own 500 - // arm is pinned in `package-list-durable-read-refusal.test.ts`. - name: 'an unexpected throw from the package service', - status: 500, - code: 'INTERNAL_ERROR', + // [#7563] The route mounts on every boot; a deployment that composes no + // `package` service answers its own 404 naming the SURFACE, never a 405 + // borrowed from a sibling pattern. + name: 'a publish on a deployment that composes no package service', + status: 404, + code: 'RESOURCE_NOT_FOUND', run: () => drive( - mount({ get: async () => { throw new Error('db down'); } }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, + mount(undefined as any), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: {} } }, ), }, ]; @@ -378,95 +276,21 @@ describe('packages envelope (#3843) — error bodies', () => { } }); - it('GET /packages no longer degrades to a 200 registry-only listing when the durable read fails (#11063)', async () => { - // REPLACED, not re-spelled. This pin used to record the opposite — a 200 - // carrying the registry half alone — described as "pre-existing, deliberate - // (`// Database query failed — continue with registry-only packages`)". It - // pinned exactly the branch #11063 removed, so re-spelling it would have - // left an assertion that passes only because nothing is produced any more. - // - // ⚠️ Note what this fixture models: a BARE `Error`. Since #10965 the real - // `PackageService.list()` swallows its own driver faults and still answers - // `[]`, and re-throws only the declared `SERVICE_UNAVAILABLE` / 503 seam - // refusal — so this shape is the UNDECLARED arm (a 500 server fault), and - // the declared-refusal arm is pinned in - // `package-list-durable-read-refusal.test.ts` alongside the `total` and - // both-doors-agree assertions. - const { status, body } = await drive( - mount({ list: async () => { throw new Error('db down'); } }, { - protocol: { getMetaItems: async () => ({ items: [{ manifest: MANIFEST }] }) }, - }), - 'GET', - PKGS, - ); - expect(status).toBe(500); - expect(body.success).toBe(false); - expect(body.error.code).toBe('INTERNAL_ERROR'); - // The registry half is not served as if it were a complete listing, and no - // `total` is reported over a read that failed. - expect(body.data?.packages).toBeUndefined(); - expect(body.data?.total).toBeUndefined(); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - }); - - it('a repeated `?version=` is refused identically on both verbs (#6307)', async () => { - // The rule is one rule, so the two verbs must answer the SAME code, status - // and message — two answers for one parameter would be a new inconsistency. - const get = await drive( - mount({ get: async () => ({ id: 'com.acme.crm', manifest: MANIFEST }) }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' }, query: { version: ['1.0.0', '2.0.0'] } }, - ); - const del = await drive( - mount({ delete: async () => ({ success: true }) }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' }, query: { version: ['1.0.0', '2.0.0'] } }, - ); - expect(get.status).toBe(400); - expect(del.status).toBe(400); - expect(get.body).toEqual(del.body); - expect(get.body.error.code).toBe('VALIDATION_ERROR'); - expect(get.body.error.message).toContain('"version"'); - expect(envelopeViolations(get.body)).toEqual([]); - expect(BaseResponseSchema.safeParse(get.body).success).toBe(true); - }); - - it('a partial uninstall keeps its per-item detail under `error.details`', async () => { - const { body } = await drive( - mount({}, { - protocol: { - deletePackage: async () => ({ - success: false, deletedCount: 1, failedCount: 2, - failed: [{ type: 'object', name: 'invoice', error: 'in use' }], - cleanups: [{ name: 'security', success: true, removed: 1 }], - }), - }, - }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - // The `failed` / `cleanups` arrays were top-level siblings of a bare - // `success: false`; they survive where the envelope declares context. - expect(body.error.details.failed).toHaveLength(1); - expect(body.error.details.cleanups).toHaveLength(1); - }); }); // ══════════════════════════════════════════════════════════════════════════════ // packages authz (#7033 / #7023) — the REST TRANSPORT's own gate // ══════════════════════════════════════════════════════════════════════════════ // -// `/packages` has TWO HTTP transports: the runtime dispatcher domain +// `/packages` had TWO HTTP transports: the runtime dispatcher domain // (`runtime/src/domains/packages.ts`, pinned in `packages-capability-gate.test.ts`) -// AND this `@objectstack/rest` direct-mount registrar — a SEPARATE handler body -// which, in the production stack, registers FIRST, so for the four routes it -// declares (`POST /publish`, `GET /`, `GET /:id`, `DELETE /:id`) it is the -// transport production actually serves. Its gate (`refusePackageRequest`) -// therefore needs its OWN pins — gating only the dispatcher would leave these -// four open, the exact one-transport gap #6603/#7019 paid for on `/meta`. +// AND this `@objectstack/rest` direct-mount registrar — a SEPARATE handler body. +// Since #14503 the registrar serves ONE route, `POST /publish`, and it is the +// only door for that verb+path, so its gate (`refusePackageRequest`) needs its +// OWN pins — gating only the dispatcher would leave it open, the exact +// one-transport gap #6603/#7019 paid for on `/meta`. The read cohort +// (`studio.access` / `setup.access`) is the dispatcher domain's to enforce and +// is pinned there; this registrar has no read route left to gate. // // Reverse check for this block: delete `refusePackageRequest`'s body and every // case below goes red. The `mount` helper above injects a gate-clearing caller @@ -482,13 +306,10 @@ describe('packages envelope (#3843) — error bodies', () => { // refusal `{ success:false, error:{ code:'FORBIDDEN' } }` (the sibling `/meta` // REST gate's code, in this surface's own wrapper). describe('packages authz (#7033 / #7023) — REST transport gate', () => { - /** A service whose four methods are spies, so "the target never ran" is an + /** A service whose method is a spy, so "the target never ran" is an * assertion, not an inference. */ const spySvc = () => ({ publish: vi.fn(async () => ({ success: true })), - list: vi.fn(async () => [{ id: 'com.acme.crm', manifest: MANIFEST }]), - get: vi.fn(async () => ({ id: 'com.acme.crm', manifest: MANIFEST })), - delete: vi.fn(async () => ({ success: true })), }); /** Mount with an EXPLICIT resolver (pass `undefined` to prove fail-closed). */ const gated = (svc: any, resolver: any) => mount(svc, { resolveExecutionContext: resolver }); @@ -498,16 +319,12 @@ describe('packages authz (#7033 / #7023) — REST transport gate', () => { type RouteShape = { method: string; path: string; body?: any; req?: Record; spy: (s: any) => any }; const PUBLISH: RouteShape = { method: 'POST', path: `${PKGS}/publish`, body: { manifest: MANIFEST, metadata: { author: 'acme' } }, spy: (s) => s.publish }; - const DELETE_ONE: RouteShape = { method: 'DELETE', path: `${PKGS}/:id`, req: { params: { id: 'com.acme.crm' }, query: { version: '1.0.0' } }, spy: (s) => s.delete }; - const LIST: RouteShape = { method: 'GET', path: PKGS, spy: (s) => s.list }; - const GET_ONE: RouteShape = { method: 'GET', path: `${PKGS}/:id`, req: { params: { id: 'com.acme.crm' } }, spy: (s) => s.get }; - const WRITE = [PUBLISH, DELETE_ONE]; - const READ = [LIST, GET_ONE]; + const WRITE = [PUBLISH]; const label = (r: RouteShape) => `${r.method} ${r.path}`; const reqOf = (r: RouteShape) => ({ ...(r.body !== undefined ? { body: r.body } : {}), ...(r.req ?? {}) }); - // ── the domain-wide anonymous floor — every route, both cohorts ── - for (const r of [...WRITE, ...READ]) { + // ── the domain-wide anonymous floor ── + for (const r of WRITE) { it(`401s an anonymous ${label(r)} (UNAUTHENTICATED) and never touches the service`, async () => { const svc = spySvc(); const c = await drive(gated(svc, anonResolver), r.method, r.path, reqOf(r)); @@ -561,49 +378,4 @@ describe('packages authz (#7033 / #7023) — REST transport gate', () => { expect(r.spy(svc)).toHaveBeenCalled(); }); } - - // ── read cohort: the ADR-0106 D4 set `studio.access` / `setup.access` ── - for (const r of READ) { - it(`403s a zero-capability caller on ${label(r)} (nested FORBIDDEN) and the service never runs`, async () => { - const svc = spySvc(); - const c = await drive(gated(svc, asCaller([])), r.method, r.path, reqOf(r)); - expect(c.status).toBe(403); - expect(c.body.error.code).toBe('FORBIDDEN'); - expect(c.body.error.message).toContain('studio.access'); - expect(r.spy(svc)).not.toHaveBeenCalled(); - }); - - it(`403s a WRITE-only caller (manage_metadata) on ${label(r)} — the read cohort is a different set`, async () => { - const svc = spySvc(); - const c = await drive(gated(svc, asCaller(['manage_metadata'])), r.method, r.path, reqOf(r)); - expect(c.status).toBe(403); - expect(c.body.error.code).toBe('FORBIDDEN'); - expect(r.spy(svc)).not.toHaveBeenCalled(); - }); - - it(`lets a studio.access caller read ${label(r)}`, async () => { - const svc = spySvc(); - const c = await drive(gated(svc, asCaller(['studio.access'])), r.method, r.path, reqOf(r)); - expect(c.status).not.toBe(403); - expect(c.status).not.toBe(401); - expect(r.spy(svc)).toHaveBeenCalled(); - }); - - it(`lets a setup.access caller read ${label(r)}`, async () => { - const svc = spySvc(); - const c = await drive(gated(svc, asCaller(['setup.access'])), r.method, r.path, reqOf(r)); - expect(c.status).not.toBe(403); - expect(c.status).not.toBe(401); - expect(r.spy(svc)).toHaveBeenCalled(); - }); - - it(`lets an isSystem caller read ${label(r)}`, async () => { - const svc = spySvc(); - const c = await drive(gated(svc, asSystem()), r.method, r.path, reqOf(r)); - expect(c.status).not.toBe(403); - expect(c.status).not.toBe(401); - expect(r.spy(svc)).toHaveBeenCalled(); - }); - } }); - diff --git a/packages/rest/src/package-id-registry-read-refusal.test.ts b/packages/rest/src/package-id-registry-read-refusal.test.ts deleted file mode 100644 index a2b211b803..0000000000 --- a/packages/rest/src/package-id-registry-read-refusal.test.ts +++ /dev/null @@ -1,388 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #11376 — `GET /api/v1/packages/:id` must not answer a terminal `404` for a - * REGISTRY read that could not happen. - * - * ## What was wrong, and why it is the WORSE half of this family - * - * The detail door tries the durable `sys_packages` read first - * (`PackageService.get()`) and falls back to the in-memory registry (via - * `protocol.getMetaItems({ type: 'package' })`). The fallback carried its own: - * - * } catch { - * // Protocol unavailable - * } - * - * so when `getMetaItems` threw, control fell straight through to the line - * below it and the door answered - * **`404 RESOURCE_NOT_FOUND` — `Package "" was not found.`** - * - * Its sibling on the list door (#11130) answered a `200` whose `total` - * under-counted. This one answers a TERMINAL NEGATIVE FACT. `404` / - * `RESOURCE_NOT_FOUND` is not "the answer may be incomplete", it is *"this - * package does not exist"*, and a caller acts on it: an installer decides the - * package is not installed and offers to install it, a console hides the entry, - * a script branches to the create path. The producer's own words for the same - * condition are the opposite — *"whether this item exists is unknown"*. - * - * Standing family ruling — #10965 · #10677 / PR #10788 · #10789 / PR #10964 · - * #11063 · #11130: **a read that could not happen must not be reported as a - * read that found nothing.** The direction here is INHERITED from those - * siblings and transplanted, not redesigned: the repair is #11063's and - * #11130's edit — delete the consumer-side catch and let the producer's - * declared refusal through `sendThrownError`. - * - * ## The producer half is already landed - * - * The live `protocol` service is `ObjectStackProtocolImplementation` - * (`packages/metadata-protocol`), whose `getMetaItems` routes every non-benign - * `sys_metadata` overlay read failure through - * `rethrowUnlessMetadataStoreUnprovisioned` → `metadataStoreUnavailableError`: - * `SERVICE_UNAVAILABLE` / 503 with an ADR-0112 status+code ON the error, pinned - * on the REAL implementation in - * `packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts` - * (#5532). This door's catch was #5532's own defect resurfacing one layer up: - * the producer was taught not to call an unreadable store "absent", and this - * consumer re-applied exactly that relabelling to the producer's answer. - * - * The refusal is reproduced locally rather than imported, for the reason both - * siblings state: this suite stays free of a cross-package VALUE import (and of - * the build-state dependence it would carry), and `packages/rest` deliberately - * has no run-time dependency on `@objectstack/metadata-protocol` at all. The - * shape it reproduces is `metadataStoreUnavailableError()` in - * `packages/metadata-protocol/src/protocol.ts`. - * - * ## What this file pins: the DISCRIMINATION, in both directions - * - * The defect is that a failed read was INDISTINGUISHABLE from an absent - * resource. Pinning only the new branch would leave "answer 500 for everything" - * passing, so §2 is as load-bearing as §1: a genuine miss, an absent protocol - * service, a registry hit and a database hit all keep the answers they had. - * §2's last case states the discrimination itself — the same request, against a - * registry that REFUSES and a registry that reads clean and empty, must not - * produce the same answer. - * - * ⚠️ No case here asserts a bare `toThrow()` or a status on its own: every - * refusal is pinned as `code` AND `status` in the ADR-0112 envelope. - * - * ⛔ No wire field is added by the fix and none is asserted here — the response - * shape is a contract decision this card does not carry. - * - * ## Anti-vacuity — directions predicted BEFORE running, measured after - * - * Baseline leg: this file run with ONLY `package-routes.ts` reverted to - * `origin/main` (the fix committed first; revert via - * `git checkout origin/main -- `, restore via - * `git checkout -- `, both under a `trap … EXIT INT TERM`), and - * the mutation confirmed ON DISK by anchored greps in both directions rather - * than by an editor's exit code. - * - * No rebuild between legs, and the claim was checked rather than assumed: the - * mutated symbol is reached by the RELATIVE import `./package-routes.js` inside - * this package, which vitest transforms from source, and the only - * `exports`-resolved workspace deps in this suite (`@objectstack/spec/api`, - * `@objectstack/types`) are untouched by the mutation. An all-green ablation - * leg would have been the stale-artifact signature; it was not what happened. - * - * §1 predicted RED 3 / GREEN 0 measured 3 red — as predicted. - * §2 predicted RED 1 / GREEN 4 measured 1 red (the discrimination case) — as - * predicted. The four controls are green on BOTH sides, which is what makes - * them controls. - * §3 predicted RED 2 / GREEN 1 measured 2 red — as predicted. The green one - * is the durable half, which has answered 503 since #10965. - * - * Total 6 of 11 red, as predicted. Predictions are left as written, per this - * repo's rule that a wrong prediction is reported rather than fitted to the - * measurement. - * - * ⚠️ One process note worth leaving for the next ablation in this package, - * because it is the exact failure the on-disk rule exists to catch: the trap's - * own restore confirmation was VOID on the first run. The `trap` fired with the - * shell's cwd inside `packages/rest`, so both the `git checkout -- - * packages/rest/src/…` restore and the `git diff --quiet -- ` check - * were given a pathspec relative to the WRONG directory — git matched nothing, - * `git diff` reported no differences, and the script printed "RESTORED" over a - * tree that was still mutated (and staged). It was caught only by re-running - * the greps from the repository root afterwards. A restore leg confirms nothing - * unless its pathspec resolves; anchor it with an absolute path or an explicit - * `-C `. - */ - -import { describe, it, expect, vi } from 'vitest'; -import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; -const PKG_ID = '/api/v1/packages/:id'; -const WANTED = 'com.acme.crm'; - -interface Captured { - status: number; - body: any; -} - -/** Only the methods these read doors reach. */ -type Svc = Partial<{ - list: () => Promise; - get: (id: string, version?: string) => Promise; -}>; - -/** - * The #5532 refusal `ObjectStackProtocolImplementation.getMetaItems` raises when - * the `sys_metadata` overlay read fails for any reason other than "the table is - * not provisioned yet", reproduced: an ADR-0112 envelope ON THE ERROR — a - * declared `status` AND a declared `code` — which is what lets it leave through - * the door's shared `resolveThrownHttpError` mapping as the PRODUCER's answer - * rather than as a 500 catch-all. - */ -function metadataStoreUnavailable(): Error { - return Object.assign( - new Error( - 'The metadata store could not be read, so whether this item exists is unknown. ' - + 'Retry once the metadata database is reachable.', - ), - { code: 'SERVICE_UNAVAILABLE', status: 503 }, - ); -} - -function mount(svc: Svc, options: any = {}) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as any; - // The authorization gate (#7033 / #7023) is not this file's subject, so the - // caller is stubbed holding the ADR-0106 D4 read set. - registerPackageRoutes(server, () => svc as any, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - ...options, - }); - return routes; -} - -async function drive( - routes: Map, - method: string, - path: string, - req: Record = {}, -): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(data: any) { captured.body = data; }, - send() {}, - status(code: number) { captured.status = code; return res; }, - header() { return res; }, - }; - await handler( - { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, - res, - ); - return captured; -} - -/** `GET /packages/:id` for the id every case below asks for. */ -const getWanted = (routes: Map) => - drive(routes, 'GET', PKG_ID, { params: { id: WANTED } }); - -/** - * A durable half that ANSWERS and does not hold the id — so the door really - * reaches the registry fallback, exactly as the defect did. `undefined` is what - * `PackageService.get()` returns for a row that is not there. - */ -const DURABLE_MISS: Svc = { get: async () => undefined }; - -/** A registry half whose PRESENT `getMetaItems` refuses with the declared 503. */ -const REFUSING_REGISTRY = { - protocol: { getMetaItems: async () => { throw metadataStoreUnavailable(); } }, -}; - -/** A registry half that reads fine and genuinely holds nothing. */ -const EMPTY_REGISTRY = { - protocol: { getMetaItems: async () => ({ items: [] }) }, -}; - -// --------------------------------------------------------------------------- -// §1 The failed REGISTRY read reaches the client -// --------------------------------------------------------------------------- - -describe('#11376 GET /packages/:id — a failed REGISTRY read is not a 404', () => { - it('answers the producer’s declared refusal (503 SERVICE_UNAVAILABLE) in the declared envelope', async () => { - const { status, body } = await getWanted(mount(DURABLE_MISS, REFUSING_REGISTRY)); - - // code AND status — the ADR-0112 envelope, never a bare `toThrow()` and - // never a status on its own. - expect(status).toBe(503); - expect(body.success).toBe(false); - expect(body.error.code).toBe('SERVICE_UNAVAILABLE'); - - // …carried in the DECLARED envelope, not an ad-hoc body. - expect(BaseResponseSchema.safeParse(body).success, JSON.stringify(body)).toBe(true); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - expect(typeof body.error.message).toBe('string'); - expect(body.error.message.length).toBeGreaterThan(0); - }); - - it('the TERMINAL negative fact is gone — no 404, no RESOURCE_NOT_FOUND, no “was not found”', async () => { - const { status, body } = await getWanted(mount(DURABLE_MISS, REFUSING_REGISTRY)); - - // The defect's exact signature, asserted as the thing that must NOT be on - // the wire: a caller told the package does not exist branches to install / - // create / hide, and none of those is a correct response to an outage. - expect(status).not.toBe(404); - expect(body.error.code).not.toBe('RESOURCE_NOT_FOUND'); - expect(JSON.stringify(body)).not.toContain('was not found'); - expect(body.data?.package).toBeUndefined(); - }); - - it('an UNDECLARED throw from the registry read is a 500 INTERNAL_ERROR, not a 404', async () => { - // The other half of "stop absorbing": a throw carrying no declared envelope - // is a server fault and now reaches the outer catch. Before this change the - // arm was unreachable on this source — the bare `catch {}` ate it too and - // answered the same 404 as a genuine miss. - const { status, body } = await getWanted(mount(DURABLE_MISS, { - protocol: { getMetaItems: async () => { throw new Error('registry exploded'); } }, - })); - - expect(status).toBe(500); - expect(body.success).toBe(false); - expect(body.error.code).toBe('INTERNAL_ERROR'); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - }); -}); - -// --------------------------------------------------------------------------- -// §2 The DISCRIMINATION the fix must preserve — the other direction -// -// Without these, "answer 503 for everything" and "answer 500 for everything" -// both pass §1. A genuine absence is still a genuine absence. -// --------------------------------------------------------------------------- - -describe('#11376 GET /packages/:id — a genuine miss is still a terminal 404', () => { - it('CONTROL — both sources read fine and neither holds the id: 404 RESOURCE_NOT_FOUND', async () => { - const { status, body } = await getWanted(mount(DURABLE_MISS, EMPTY_REGISTRY)); - - expect(status).toBe(404); - expect(body.success).toBe(false); - expect(body.error.code).toBe('RESOURCE_NOT_FOUND'); - expect(body.error.message).toContain(WANTED); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - }); - - it('CONTROL — an ABSENT protocol service is an absence, not a failed read: still 404', async () => { - // The overreach guard. `if (options.protocol && typeof … === 'function')` - // answers "no registry in this composition", which is a fact about the - // deployment rather than a read that failed. A fix that turned those - // deployments into 503s would break every one of them; nothing about that - // path moved. - const { status, body } = await getWanted(mount(DURABLE_MISS, {})); - - expect(status).toBe(404); - expect(body.error.code).toBe('RESOURCE_NOT_FOUND'); - }); - - it('CONTROL — a registry HIT still answers 200 with `source: "registry"`', async () => { - const { status, body } = await getWanted(mount(DURABLE_MISS, { - protocol: { - getMetaItems: async () => ({ items: [{ manifest: { id: WANTED, version: '1.0.0' } }] }), - }, - })); - - expect(status).toBe(200); - expect(body.success).toBe(true); - expect(body.data.package.source).toBe('registry'); - expect(body.data.package.manifest.id).toBe(WANTED); - }); - - it('CONTROL — a DATABASE hit still answers 200 and never consults the registry', async () => { - // The durable read is tried first and short-circuits. Pinned with a spy so - // "the registry is not even asked" is measured, not inferred: a refusing - // registry must not be able to turn a successful durable read into a 503. - const getMetaItems = vi.fn(async () => { throw metadataStoreUnavailable(); }); - const { status, body } = await getWanted(mount( - { get: async () => ({ id: WANTED, version: '2.0.0', manifest: { id: WANTED, version: '2.0.0' } }) }, - { protocol: { getMetaItems } }, - )); - - expect(status).toBe(200); - expect(body.success).toBe(true); - expect(body.data.package.source).toBe('database'); - expect(getMetaItems).not.toHaveBeenCalled(); - }); - - it('the DISCRIMINATION itself: an outage and a clean-empty registry no longer answer alike', async () => { - // One request, two registries. This is the defect stated directly — before - // the fix both of these were `404 RESOURCE_NOT_FOUND` and a caller had - // nothing on the wire to tell "this package does not exist" from "I could - // not find out". Asserted as a DIFFERENCE so it cannot be satisfied by - // moving both answers together. - const outage = await getWanted(mount(DURABLE_MISS, REFUSING_REGISTRY)); - const miss = await getWanted(mount(DURABLE_MISS, EMPTY_REGISTRY)); - - expect(outage.status).not.toBe(miss.status); - expect(outage.body.error.code).not.toBe(miss.body.error.code); - expect(miss.body.error.code).toBe('RESOURCE_NOT_FOUND'); - expect(outage.body.error.code).toBe('SERVICE_UNAVAILABLE'); - }); -}); - -// --------------------------------------------------------------------------- -// §3 One outage, one answer — across the halves of this door and across doors -// --------------------------------------------------------------------------- - -describe('#11376 the same metadata-store outage answers identically everywhere', () => { - it('CONTROL — the DURABLE half of this door already answered 503, and still does', async () => { - // Green before and after: `PackageService.get()` has never had an inner - // catch, so its #10965 refusal has reached `sendThrownError` all along. - // That is also the measurement behind this card's `Clause-②` answer — 503 - // was already a reachable answer on this exact route, so letting the - // registry half reach it too adds no status to the door's answer set. - const { status, body } = await getWanted(mount( - { get: async () => { throw metadataStoreUnavailable(); } }, - EMPTY_REGISTRY, - )); - - expect(status).toBe(503); - expect(body.error.code).toBe('SERVICE_UNAVAILABLE'); - }); - - it('both HALVES of this door answer the same outage identically', async () => { - // Which of the two stores is down is an implementation detail of the - // merge, never a fact about the caller's request. Agreement is the fix. - const registryHalf = await getWanted(mount(DURABLE_MISS, REFUSING_REGISTRY)); - const durableHalf = await getWanted(mount( - { get: async () => { throw metadataStoreUnavailable(); } }, - EMPTY_REGISTRY, - )); - - expect(registryHalf.status).toBe(durableHalf.status); - expect(registryHalf.body.error.code).toBe(durableHalf.body.error.code); - expect(registryHalf.body.success).toBe(durableHalf.body.success); - }); - - it('the DETAIL door and the LIST door answer the same registry outage identically', async () => { - // #11130 fixed the list door's registry half; the detail door answering a - // terminal 404 for the same outage was the surviving inconsistency — and - // the more dangerous of the two answers. - const detail = await getWanted(mount(DURABLE_MISS, REFUSING_REGISTRY)); - const list = await drive( - mount({ list: async () => [] }, REFUSING_REGISTRY), - 'GET', - PKGS, - ); - - expect(detail.status).toBe(list.status); - expect(detail.body.error.code).toBe(list.body.error.code); - }); -}); diff --git a/packages/rest/src/package-list-durable-read-refusal.test.ts b/packages/rest/src/package-list-durable-read-refusal.test.ts deleted file mode 100644 index 214ca8261b..0000000000 --- a/packages/rest/src/package-list-durable-read-refusal.test.ts +++ /dev/null @@ -1,245 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #11063 — `GET /api/v1/packages` must not absorb a failed durable read. - * - * ## What was wrong, and why "still 200" was not a pin - * - * The list door merged two sources — the in-memory registry (via - * `protocol.getMetaItems`) and the durable `sys_packages` rows (via - * `PackageService.list()`) — and wrapped the durable half in a bare - * `catch {}` commented *"Database query failed — continue with registry-only - * packages"*. A failed durable read was therefore reported as a 200 whose - * `total` claimed to be a COMPLETE count, and whose registrar-sourced entries - * kept `source: 'registry'` — which reads as PROVENANCE, not as a warning that - * the database half is absent. Nothing on the wire separated *"these are all - * the packages"* from *"these are the packages I could still see"*. - * - * This is the standing family ruling — #10965 · #10677 / PR #10788 · #10789 / - * PR #10964: **a read that could not happen must not be reported as a read that - * found nothing.** Here it sat one level up, in a consumer-side catch rather - * than in a flattener, which is why the producer-side fix could not close it. - * - * ⚠️ Asserting "the listing returns 200" passes on the OLD code, on the fixed - * code, and on a wrong fix — it is the empty assertion this file exists to - * avoid. Every case below pins the MECHANISM instead: which status and which - * declared `code` reach the client when the durable read refuses, that `total` - * is not reported at all over a read that failed, and that the two read doors - * answer the same failure identically. - * - * ## Where the halves are pinned - * - * The PRODUCER half — that `PackageService.list()`/`get()` refuse with - * `SERVICE_UNAVAILABLE` / 503 over a seam that accepted the query and returned - * no result set — is measured on a real booted engine in - * `packages/runtime/src/package-service.null-seam.test.ts` (#10965). This file - * pins the DOOR half: that the declared refusal travels through the REST - * envelope instead of being swallowed. The refusal is reproduced locally rather - * than imported so this suite stays free of a cross-package VALUE import (and - * of the build-state dependence one would carry — `@objectstack/service-package` - * is not aliased to `src/` in this package's vitest config); the shape it - * reproduces is `packageSeamUnreadableError()` in - * `packages/services/service-package/src/index.ts`. - * - * ⛔ No wire field is added by the fix and none is asserted here. The card's - * alternative — keep the 200 and carry a declared partial-result marker — is a - * response-shape change, i.e. a contract decision, and was not authorized. - */ - -import { describe, it, expect } from 'vitest'; -import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; - -interface Captured { - status: number; - body: any; -} - -/** Only the methods these two read doors reach. */ -type Svc = Partial<{ - list: () => Promise; - get: (id: string, version?: string) => Promise; -}>; - -/** - * The #10965 refusal, reproduced: an ADR-0112 envelope ON THE ERROR — a - * declared `status` AND a declared `code` — which is what lets it leave through - * the door's shared `resolveThrownHttpError` mapping as the PRODUCER's answer - * rather than as a 500 catch-all. - */ -function seamUnreadableError(): Error { - return Object.assign( - new Error( - 'The package registry could not be read: the storage seam accepted the query but returned no ' - + 'result set. Whether this package is installed is UNKNOWN — this is not an answer of "no".', - ), - { code: 'SERVICE_UNAVAILABLE', status: 503 }, - ); -} - -function mount(svc: Svc, options: any = {}) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as any; - // The authorization gate (#7033 / #7023) is not this file's subject, so the - // caller is stubbed holding the ADR-0106 D4 read set. - registerPackageRoutes(server, () => svc as any, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - ...options, - }); - return routes; -} - -async function drive( - routes: Map, - method: string, - path: string, - req: Record = {}, -): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(data: any) { captured.body = data; }, - send() {}, - status(code: number) { captured.status = code; return res; }, - header() { return res; }, - }; - await handler( - { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, - res, - ); - return captured; -} - -const REGISTRY_MANIFEST = { id: 'com.acme.registry-only', version: '1.0.0' }; - -/** A registry half that DOES answer — so a swallowed durable failure would have - * something to answer 200 with, exactly as the defect did. */ -const REGISTRY_PROTOCOL = { - protocol: { getMetaItems: async () => ({ items: [{ manifest: REGISTRY_MANIFEST }] }) }, -}; - -describe('#11063 GET /packages — a failed durable read reaches the client', () => { - it('answers the producer’s declared refusal (503 SERVICE_UNAVAILABLE), not a 200', async () => { - const { status, body } = await drive( - mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), - 'GET', - PKGS, - ); - - // code AND status — the ADR-0112 envelope, never a bare `toThrow()` and - // never a status on its own. - expect(status).toBe(503); - expect(body.success).toBe(false); - expect(body.error.code).toBe('SERVICE_UNAVAILABLE'); - - // …carried in the DECLARED envelope, not an ad-hoc body. - expect(BaseResponseSchema.safeParse(body).success, JSON.stringify(body)).toBe(true); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - expect(typeof body.error.message).toBe('string'); - expect(body.error.message.length).toBeGreaterThan(0); - }); - - it('reports NO `total` over a read that failed — the corrupted complete count is gone', async () => { - const { status, body } = await drive( - mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), - 'GET', - PKGS, - ); - - // The defect's signature: a `total` presented as a complete count while the - // durable half was missing, and a `packages` array the caller could not - // tell apart from a full listing. - expect(status).not.toBe(200); - expect(body.data?.total).toBeUndefined(); - expect(body.data?.packages).toBeUndefined(); - - // And specifically NOT the registry-only listing served as if it were whole. - expect(body.data?.packages).not.toEqual([ - expect.objectContaining({ source: 'registry' }), - ]); - }); - - it('answers the SAME failure identically on both read doors (#11063 alignment)', async () => { - // `GET /packages/:id` has never had an inner catch, so it has answered this - // refusal since #10965. The list door disagreeing with it WAS the defect; - // agreement is the fix, and it is worth one assertion. - const list = await drive( - mount({ list: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), - 'GET', - PKGS, - ); - const detail = await drive( - mount({ get: async () => { throw seamUnreadableError(); } }, REGISTRY_PROTOCOL), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - - expect(list.status).toBe(detail.status); - expect(list.body.error.code).toBe(detail.body.error.code); - expect(list.body.success).toBe(detail.body.success); - }); - - it('an UNDECLARED throw from the durable read is a 500 INTERNAL_ERROR, not a 200', async () => { - // The other half of "stop absorbing": a throw carrying no declared envelope - // is a server fault and now reaches the outer catch. Before the fix this - // arm was unreachable on this route — which is why the sibling envelope - // suite had to drive `GET /:id` to exercise it at all. - const { status, body } = await drive( - mount({ list: async () => { throw new Error('db down'); } }, REGISTRY_PROTOCOL), - 'GET', - PKGS, - ); - - expect(status).toBe(500); - expect(body.success).toBe(false); - expect(body.error.code).toBe('INTERNAL_ERROR'); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - }); - - it('a durable read that ANSWERS still merges both sources and counts them truthfully', async () => { - // The half that keeps this from being "refuse always": nothing about the - // healthy path moved. Two sources, one overlapping id, and a `total` that - // is a real complete count of what was really read. - const { status, body } = await drive( - mount( - { - list: async () => [ - { id: 'com.acme.registry-only', version: '1.0.0', manifest: REGISTRY_MANIFEST }, - { id: 'com.acme.published', version: '2.0.0', manifest: { id: 'com.acme.published' } }, - ], - }, - REGISTRY_PROTOCOL, - ), - 'GET', - PKGS, - ); - - expect(status).toBe(200); - expect(body.success).toBe(true); - expect(body.data.total).toBe(2); - expect(body.data.packages).toHaveLength(2); - - const bySource = Object.fromEntries( - body.data.packages.map((p: any) => [p.manifest?.id ?? p.id, p.source]), - ); - // The id both halves carry is `both`; the durable-only id is `database`. - expect(bySource['com.acme.registry-only']).toBe('both'); - expect(bySource['com.acme.published']).toBe('database'); - }); -}); diff --git a/packages/rest/src/package-list-registry-read-refusal.test.ts b/packages/rest/src/package-list-registry-read-refusal.test.ts deleted file mode 100644 index e1aafe300e..0000000000 --- a/packages/rest/src/package-list-registry-read-refusal.test.ts +++ /dev/null @@ -1,279 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #11130 — `GET /api/v1/packages` must not absorb a failed REGISTRY read either. - * - * ## What was wrong - * - * The list door merges TWO sources — the in-memory registry (via - * `protocol.getMetaItems({ type: 'package' })`) and the durable `sys_packages` - * rows (via `PackageService.list()`). #11063 stopped the door absorbing a - * failure of the DURABLE half. The REGISTRY half still carried its own: - * - * } catch { - * // Protocol unavailable — continue with database only - * } - * - * so the exact ambiguity #11063 closed stayed open on the other half: when - * `getMetaItems` threw, the door answered `200` with `{ packages, total }` built - * from the database alone, and `total` was reported as a COMPLETE count either - * way. The surviving entries kept `source: 'database'`, which reads as - * PROVENANCE, not as a warning that the registry half is absent. Nothing on the - * wire separated *"these are all the packages"* from *"these are the packages I - * could still see"*. - * - * Standing family ruling — #10965 · #10677 / PR #10788 · #10789 / PR #10964 · - * #11063: **a read that could not happen must not be reported as a read that - * found nothing.** - * - * ## Why the producer half of route (b) is already landed - * - * The card's route (b) is "teach the producer to declare a refusal, then stop - * swallowing it". Measured before this change: the producer ALREADY declares - * it. The live `protocol` service is - * `ObjectStackProtocolImplementation` (`packages/metadata-protocol`), whose - * `getMetaItems` routes every non-benign `sys_metadata` read failure through - * `rethrowUnlessMetadataStoreUnprovisioned` → `metadataStoreUnavailableError`, - * i.e. `SERVICE_UNAVAILABLE` / 503 with an ADR-0112 status+code on the error — - * the same envelope #10965 gave `PackageService.list()`. That producer behaviour - * is pinned on the REAL implementation in - * `packages/metadata-protocol/src/protocol.metadata-store-outage.test.ts` - * (#5532). So the only leg left for this door is #11063's: stop swallowing. - * - * ⚠️ Asserting "the listing returns 200" passes on the OLD code, on the fixed - * code, and on a wrong fix. Every case below pins the MECHANISM instead: which - * status and which declared `code` reach the client, that `total` is not - * reported at all over a read that failed, and that the two HALVES of one merge - * answer the same failure identically. - * - * The refusal is reproduced locally rather than imported, for the reason the - * #11063 sibling states: this suite stays free of a cross-package VALUE import - * (and of the build-state dependence it would carry) — and `packages/rest` - * deliberately has no run-time dependency on `@objectstack/metadata-protocol` - * at all. The shape it reproduces is `metadataStoreUnavailableError()` in - * `packages/metadata-protocol/src/protocol.ts`. - * - * ⛔ No wire field is added by the fix and none is asserted here. Shape (c) — - * keep the 200 and make the tolerance visible with a partial-result marker — is - * a response-shape change, i.e. a contract decision this queue entry does not - * carry. - */ - -import { describe, it, expect } from 'vitest'; -import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; - -interface Captured { - status: number; - body: any; -} - -/** Only the methods this read door reaches. */ -type Svc = Partial<{ - list: () => Promise; - get: (id: string, version?: string) => Promise; -}>; - -/** - * The #5532 refusal `ObjectStackProtocolImplementation.getMetaItems` raises when - * the `sys_metadata` overlay read fails for any reason other than "the table is - * not provisioned yet", reproduced: an ADR-0112 envelope ON THE ERROR — a - * declared `status` AND a declared `code` — which is what lets it leave through - * the door's shared `resolveThrownHttpError` mapping as the PRODUCER's answer - * rather than as a 500 catch-all. - */ -function metadataStoreUnavailable(): Error { - return Object.assign( - new Error( - 'The metadata store could not be read, so whether this item exists is unknown. ' - + 'Retry once the metadata database is reachable.', - ), - { code: 'SERVICE_UNAVAILABLE', status: 503 }, - ); -} - -function mount(svc: Svc, options: any = {}) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as any; - // The authorization gate (#7033 / #7023) is not this file's subject, so the - // caller is stubbed holding the ADR-0106 D4 read set. - registerPackageRoutes(server, () => svc as any, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - ...options, - }); - return routes; -} - -async function drive( - routes: Map, - method: string, - path: string, - req: Record = {}, -): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(data: any) { captured.body = data; }, - send() {}, - status(code: number) { captured.status = code; return res; }, - header() { return res; }, - }; - await handler( - { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, - res, - ); - return captured; -} - -const DB_MANIFEST = { id: 'com.acme.published', version: '2.0.0' }; - -/** A durable half that DOES answer — so a swallowed registry failure has - * something to answer 200 with, exactly as the defect did. */ -const DURABLE_ROWS: Svc = { - list: async () => [{ id: 'com.acme.published', version: '2.0.0', manifest: DB_MANIFEST }], -}; - -/** A registry half whose PRESENT `getMetaItems` refuses with the declared 503. */ -const REFUSING_REGISTRY = { - protocol: { getMetaItems: async () => { throw metadataStoreUnavailable(); } }, -}; - -describe('#11130 GET /packages — a failed REGISTRY read reaches the client', () => { - it('answers the producer’s declared refusal (503 SERVICE_UNAVAILABLE), not a 200', async () => { - const { status, body } = await drive(mount(DURABLE_ROWS, REFUSING_REGISTRY), 'GET', PKGS); - - // code AND status — the ADR-0112 envelope, never a bare `toThrow()` and - // never a status on its own. - expect(status).toBe(503); - expect(body.success).toBe(false); - expect(body.error.code).toBe('SERVICE_UNAVAILABLE'); - - // …carried in the DECLARED envelope, not an ad-hoc body. - expect(BaseResponseSchema.safeParse(body).success, JSON.stringify(body)).toBe(true); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - expect(typeof body.error.message).toBe('string'); - expect(body.error.message.length).toBeGreaterThan(0); - }); - - it('reports NO `total` over a read that failed — the corrupted complete count is gone', async () => { - const { status, body } = await drive(mount(DURABLE_ROWS, REFUSING_REGISTRY), 'GET', PKGS); - - // The defect's signature: a `total` presented as a complete count while the - // registry half was missing, and a `packages` array the caller could not - // tell apart from a full listing. - expect(status).not.toBe(200); - expect(body.data?.total).toBeUndefined(); - expect(body.data?.packages).toBeUndefined(); - - // And specifically NOT the database-only listing served as if it were whole. - expect(body.data?.packages).not.toEqual([ - expect.objectContaining({ source: 'database' }), - ]); - }); - - it('answers the SAME failure identically whichever HALF of the merge refuses', async () => { - // One door, two sources. #11063 made the durable half answer the producer's - // refusal; a registry half that still swallowed meant the SAME outage got - // two different answers depending on which store was down. Agreement is the - // fix, and it is worth one assertion. - const registryHalf = await drive(mount(DURABLE_ROWS, REFUSING_REGISTRY), 'GET', PKGS); - const durableHalf = await drive( - mount( - { list: async () => { throw metadataStoreUnavailable(); } }, - { protocol: { getMetaItems: async () => ({ items: [] }) } }, - ), - 'GET', - PKGS, - ); - - expect(registryHalf.status).toBe(durableHalf.status); - expect(registryHalf.body.error.code).toBe(durableHalf.body.error.code); - expect(registryHalf.body.success).toBe(durableHalf.body.success); - }); - - it('an UNDECLARED throw from the registry read is a 500 INTERNAL_ERROR, not a 200', async () => { - // The other half of "stop absorbing": a throw carrying no declared envelope - // is a server fault and now reaches the outer catch. Before this change the - // arm was unreachable on this source — the bare `catch {}` ate it too. - const { status, body } = await drive( - mount(DURABLE_ROWS, { - protocol: { getMetaItems: async () => { throw new Error('registry exploded'); } }, - }), - 'GET', - PKGS, - ); - - expect(status).toBe(500); - expect(body.success).toBe(false); - expect(body.error.code).toBe('INTERNAL_ERROR'); - expect(envelopeViolations(body), JSON.stringify(body)).toEqual([]); - }); - - it('an ABSENT protocol service is still the different, HANDLED case — 200, database only', async () => { - // The overreach guard. The `if (options.protocol && typeof … === 'function')` - // guard answers "no registry here", which is a fact, not a failed read; a - // fix that turned a composition without the protocol service into a 503 - // would break every such deployment. Nothing about that path moved. - const { status, body } = await drive(mount(DURABLE_ROWS, {}), 'GET', PKGS); - - expect(status).toBe(200); - expect(body.success).toBe(true); - expect(body.data.total).toBe(1); - expect(body.data.packages[0].source).toBe('database'); - }); - - it('a registry read that ANSWERS still merges both sources and counts them truthfully', async () => { - // The half that keeps this from being "refuse always": nothing about the - // healthy path moved. Two sources, one overlapping id, and a `total` that is - // a real complete count of what was really read. - const { status, body } = await drive( - mount( - { - list: async () => [ - { id: 'com.acme.published', version: '2.0.0', manifest: DB_MANIFEST }, - { id: 'com.acme.both', version: '1.0.0', manifest: { id: 'com.acme.both' } }, - ], - }, - { - protocol: { - getMetaItems: async () => ({ - items: [ - { manifest: { id: 'com.acme.both', version: '1.0.0' } }, - { manifest: { id: 'com.acme.registry-only', version: '1.0.0' } }, - ], - }), - }, - }, - ), - 'GET', - PKGS, - ); - - expect(status).toBe(200); - expect(body.success).toBe(true); - expect(body.data.total).toBe(3); - expect(body.data.packages).toHaveLength(3); - - const bySource = Object.fromEntries( - body.data.packages.map((p: any) => [p.manifest?.id ?? p.id, p.source]), - ); - expect(bySource['com.acme.registry-only']).toBe('registry'); - expect(bySource['com.acme.published']).toBe('database'); - expect(bySource['com.acme.both']).toBe('both'); - }); -}); diff --git a/packages/rest/src/package-list-writable-carry.test.ts b/packages/rest/src/package-list-writable-carry.test.ts deleted file mode 100644 index 142822326a..0000000000 --- a/packages/rest/src/package-list-writable-carry.test.ts +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * REST `GET /packages` CARRIES the producer's `writable` verdict (#14375). - * - * The REST list door does not compute writability itself — it must not: this - * package has no runtime dependency on `@objectstack/metadata-protocol`, and - * the verdict has one definition (`isWritablePackage`, ADR-0070 D2) that the - * protocol's `getMetaItems({ type: 'package' })` now stamps on every registry - * item. What THIS door owns is the merge: the durable (`PackageService.list()`) - * row is spread OVER the registry item, so a durable row that carries no - * `writable` key must leave the registry item's verdict standing, and a - * durable-only row (no registry presence) carries no verdict at all. Both are - * pinned here, because the spread order is the one place this file could lose - * the field. - */ - -import { describe, it, expect } from 'vitest'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { registerPackageRoutes } from './package-routes.js'; - -type Captured = { status: number; body: any }; - -function mount(svc: any, protocol: any) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as any; - // The authorization gate (#7033 / #7023) is not this file's subject. - registerPackageRoutes(server, () => svc, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - protocol, - }); - return routes; -} - -async function drive(routes: Map, method: string, path: string, req: Record = {}): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(data: any) { captured.body = data; }, - send() {}, - status(code: number) { captured.status = code; return res; }, - header() { return res; }, - }; - await handler({ params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, res); - return captured; -} - -/** The scope-less booted module — the producer says read-only. */ -const MODULE = 'app.acme.crm.billing'; -/** A scope-less Studio base — the producer says writable; it also has a durable row. */ -const BASE = 'com.acme.mybase'; -/** A durable-only row: published, never registered on this process. */ -const DURABLE_ONLY = 'com.acme.published-elsewhere'; - -const registryItems = [ - { manifest: { id: MODULE, name: MODULE, version: '1.0.0', type: 'module' }, status: 'installed', enabled: true, writable: false }, - { manifest: { id: BASE, name: BASE, version: '1.0.0' }, status: 'installed', enabled: true, writable: true }, -]; -const protocol = { getMetaItems: async () => ({ type: 'package', items: registryItems }) }; -const svc = { - list: async () => [ - // The durable row for BASE has NO `writable` key — a durable copy is not - // where the verdict lives. - { id: BASE, version: '1.0.0', manifest: { id: BASE, name: BASE, version: '1.0.0' } }, - { id: DURABLE_ONLY, version: '3.0.0', manifest: { id: DURABLE_ONLY, version: '3.0.0' } }, - ], -}; - -const rowOf = (body: any, id: string) => body.data.packages.find((p: any) => (p.manifest?.id ?? p.id) === id); - -describe('REST GET /packages carries the producer\'s writable verdict through the merge (#14375)', () => { - it('a registry-only row keeps the producer\'s verdict', async () => { - const r = await drive(mount(svc, protocol), 'GET', '/api/v1/packages'); - expect(r.status).toBe(200); - const row = rowOf(r.body, MODULE); - expect(row.source).toBe('registry'); - expect(row.writable).toBe(false); - }); - - it('a durable row spread over a registry item does NOT erase the verdict (spread order)', async () => { - const r = await drive(mount(svc, protocol), 'GET', '/api/v1/packages'); - const row = rowOf(r.body, BASE); - expect(row.source).toBe('both'); - // The durable row carried no `writable`; the registry item's stands. - expect(row.writable).toBe(true); - }); - - it('a durable-only row carries no verdict — the registry item is the only carrier', async () => { - const r = await drive(mount(svc, protocol), 'GET', '/api/v1/packages'); - const row = rowOf(r.body, DURABLE_ONLY); - expect(row.source).toBe('database'); - expect('writable' in row).toBe(false); - }); - - it('is additive: nothing else about the merged rows changed', async () => { - const r = await drive(mount(svc, protocol), 'GET', '/api/v1/packages'); - expect(r.body.data.total).toBe(3); - const { writable, ...rest } = rowOf(r.body, MODULE); - expect(writable).toBe(false); - const { writable: _producerVerdict, ...producerItem } = registryItems[0]; - expect(rest).toEqual({ ...producerItem, source: 'registry' }); - }); -}); diff --git a/packages/rest/src/package-publish-mount.test.ts b/packages/rest/src/package-publish-mount.test.ts index d95465e1d6..875a004291 100644 --- a/packages/rest/src/package-publish-mount.test.ts +++ b/packages/rest/src/package-publish-mount.test.ts @@ -28,13 +28,16 @@ * to a sibling's 405. It therefore mounts unconditionally and answers its * own honest 404 where no package service exists. * - * ## The asymmetry is load-bearing, and is pinned too + * ## The registrar mounts publish and NOTHING ELSE — pinned too (#14503) * - * The other three package routes must NOT follow. Each shadows a live - * dispatcher twin at a byte-identical pattern; mounting them without a service - * would replace three working routes with a degraded refusal. A future edit - * that "makes the registrar consistent" by mounting all four unconditionally - * fails the `keeps its hands off the three dispatcher twins` case below. + * The three routes that used to sit beside publish behind a `package`-service + * gate — `GET /packages`, `GET /packages/:id`, `DELETE /packages/:id` — are + * gone from this registrar: they duplicated the dispatcher's `/packages` + * domain at byte-identical patterns, the two had already diverged (404 + * wording, `{ package }` wrapper, `source` stamp), and the ruling on #14503 + * made the dispatcher domain the single implementation. A future edit that + * "restores" any of them here, with or without a service, fails the + * `mounts nothing but publish` cases below. */ // `.js` on the relative imports: under `moduleResolution: nodenext` an @@ -123,30 +126,27 @@ describe('#7563 — POST /packages/publish is mounted with or without a `package expect(returned.map((r) => `${r.method} ${r.path}`)).toEqual(['POST /api/v1/packages/publish']); }); - it('keeps its hands off the three dispatcher twins when there is no service', () => { - // The asymmetry, stated as a test so it cannot be "tidied up": these three - // patterns are served by the runtime dispatcher on a package-service-less - // stack, and a degraded REST shadow registered ahead of them would take - // three working routes away. + it('mounts nothing but publish when there is no service — the three former twins are the dispatcher\'s alone (#14503)', () => { + // Stated as a test so it cannot be "tidied up" back: these three patterns + // are served by the runtime dispatcher's `/packages` domain, the single + // implementation since #14503, and a REST copy registered beside it would + // re-create the two-doors-one-URL fork this card closed. const server = createMockServer(); registerPackageRoutes(server as any, () => undefined, '/api/v1', AUTHED); const keys = mountedOn(server); + expect(keys).toEqual(['POST /api/v1/packages/publish']); expect(keys).not.toContain('GET /api/v1/packages'); expect(keys).not.toContain('GET /api/v1/packages/:id'); expect(keys).not.toContain('DELETE /api/v1/packages/:id'); }); - it('mounts the full surface when a package service is there', () => { + it('mounts nothing but publish when a package service IS there — presence no longer changes the surface', () => { const server = createMockServer(); - registerPackageRoutes(server as any, () => packageServiceStub() as any, '/api/v1', AUTHED); + const returned = registerPackageRoutes(server as any, () => packageServiceStub() as any, '/api/v1', AUTHED); - expect(mountedOn(server).sort()).toEqual([ - 'DELETE /api/v1/packages/:id', - 'GET /api/v1/packages', - 'GET /api/v1/packages/:id', - 'POST /api/v1/packages/publish', - ]); + expect(mountedOn(server)).toEqual(['POST /api/v1/packages/publish']); + expect(returned.map((r) => `${r.method} ${r.path}`)).toEqual(['POST /api/v1/packages/publish']); }); }); @@ -246,14 +246,9 @@ describe('#7563 — the composition mounts publish on a service-less boot', () = expect(recorded.filter((r) => r.includes('/packages'))).toEqual(['POST /api/v1/packages/publish']); }); - it('records all four when `package` is present', () => { + it('records the publish route — and only it — when `package` is present too (#14503)', () => { const { recorded } = compose({ package: packageServiceStub() }); - expect(recorded.filter((r) => r.includes('/packages')).sort()).toEqual([ - 'DELETE /api/v1/packages/:id', - 'GET /api/v1/packages', - 'GET /api/v1/packages/:id', - 'POST /api/v1/packages/publish', - ]); + expect(recorded.filter((r) => r.includes('/packages'))).toEqual(['POST /api/v1/packages/publish']); }); it('mirrors the publish route under the scoped base, on both bases, with no service', () => { diff --git a/packages/rest/src/package-registry-item-projection.test.ts b/packages/rest/src/package-registry-item-projection.test.ts deleted file mode 100644 index 0cc07b22d4..0000000000 --- a/packages/rest/src/package-registry-item-projection.test.ts +++ /dev/null @@ -1,200 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * The REST package read doors project a REGISTRY-sourced entry onto its - * declared fields instead of spreading it whole. - * - * ## Where this sits - * - * The card behind it is the `500 Converting circular structure to JSON` a stock - * showcase boot answered on `GET /api/v1/packages` — `SchemaRegistry.install- - * Package` stored the caller's live `defineStack()` object, whose `plugins: [...]` - * held initialised plugin instances and through them the engine. The REPAIR is - * at that producer (`@objectstack/objectql`), so by the time an entry reaches - * this door there is nothing unserializable left in it. - * - * ⚠️ Measured, and worth writing down because the card attributed the 500 here: - * in the showcase composition these two same-pattern routes are NOT the ones - * that answered — the dispatcher twin (`packages/runtime/src/domains/packages.ts`) - * did. The 404 wording separates them: `Package "x" was not found.` here, - * `Package 'x' not found` there, and the live probe returned the latter. So what - * this file pins is this door's own posture, not the reproduction of the boot. - * - * ## What it pins - * - * `{ ...item, source: 'registry' }` gave ONE undeclared member on ONE package - * the power to fail the whole list for every caller. The projection turns that - * into a field the response never mentions. Both halves matter and both are - * asserted: the undeclared member is DROPPED, and every declared field — - * including the `_diagnostics` the protocol's own decoration grafts on — is - * still SERVED. A projection that quietly ate `_diagnostics` would pass a - * "no longer 500" assertion just as well. - * - * ⛔ The DATABASE half is deliberately not projected and is asserted unchanged: - * its shape belongs to `PackageService`, not to this door. - */ - -import { describe, it, expect } from 'vitest'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; - -interface Captured { status: number; body: any } - -/** The engine's own `actionActivation -> store -> engine` cycle, reproduced. */ -function cyclicEngine(): Record { - const engine: Record = { name: '_ObjectQL' }; - const store: Record = { name: 'ObjectStoreActionActivationStore', engine }; - engine.actionActivation = { name: 'ActionActivationProjection', store }; - return engine; -} - -/** - * A registry entry in the shape `protocol.getMetaItems({ type: 'package' })` - * yields: the installed-package record, plus the `_diagnostics` that - * `decorateMetadataItem` grafts on, plus — the case under test — an undeclared - * member holding a live object. - */ -function registryEntry(extra: Record = {}) { - return { - manifest: { id: 'com.example.showcase', name: 'Showcase', version: '0.3.16' }, - status: 'installed', - enabled: true, - installedAt: '2026-09-02T00:00:00.000Z', - updatedAt: '2026-09-02T00:00:00.000Z', - settings: { theme: 'dark' }, - _diagnostics: [{ code: 'SOMETHING', message: 'noted' }], - ...extra, - }; -} - -function mount(svc: any, protocolItems: unknown[]) { - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as any; - // The authorization gate (#7033 / #7023) is not this file's subject. - registerPackageRoutes(server, () => svc as any, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - protocol: { getMetaItems: async () => ({ items: protocolItems }) }, - } as any); - return routes; -} - -async function drive( - routes: Map, - method: string, - path: string, - req: Record = {}, -): Promise { - const handler = routes.get(`${method}:${path}`); - if (!handler) throw new Error(`no handler for ${method} ${path}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(data: any) { captured.body = data; }, - send() {}, - status(code: number) { captured.status = code; return res; }, - header() { return res; }, - }; - await handler( - { params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any, - res, - ); - return captured; -} - -/** A durable half that finds nothing, so only the registry half is exercised. */ -const EMPTY_DB = { list: async () => [], get: async () => null }; - -describe('GET /packages — registry entries are projected, not spread', () => { - it('serves the entry when it carries an undeclared LIVE member, instead of 500', async () => { - const routes = mount(EMPTY_DB, [registryEntry({ liveEngineHandle: cyclicEngine() })]); - const { status, body } = await drive(routes, 'GET', PKGS); - - expect(status).toBe(200); - // The step the transport takes next, and the one that threw on the boot. - expect(() => JSON.stringify(body)).not.toThrow(); - expect(body.data.packages).toHaveLength(1); - expect('liveEngineHandle' in body.data.packages[0]).toBe(false); - }); - - it('still serves every declared field, and the protocol’s `_diagnostics`', async () => { - const routes = mount(EMPTY_DB, [registryEntry({ liveEngineHandle: cyclicEngine() })]); - const { body } = await drive(routes, 'GET', PKGS); - const served = body.data.packages[0]; - - expect(served.manifest).toEqual({ id: 'com.example.showcase', name: 'Showcase', version: '0.3.16' }); - expect(served.status).toBe('installed'); - expect(served.enabled).toBe(true); - expect(served.installedAt).toBe('2026-09-02T00:00:00.000Z'); - expect(served.updatedAt).toBe('2026-09-02T00:00:00.000Z'); - expect(served.settings).toEqual({ theme: 'dark' }); - expect(served._diagnostics).toEqual([{ code: 'SOMETHING', message: 'noted' }]); - // The provenance marker the door adds is unchanged. - expect(served.source).toBe('registry'); - }); - - it('omits a declared field that is absent rather than serialising `undefined`', async () => { - const routes = mount(EMPTY_DB, [{ - manifest: { id: 'com.objectstack.setup' }, status: 'installed', enabled: true, - }]); - const { body } = await drive(routes, 'GET', PKGS); - - expect(Object.keys(body.data.packages[0]).sort()) - .toEqual(['enabled', 'manifest', 'source', 'status']); - }); - - it('leaves the DATABASE half’s shape alone — this door does not own it', async () => { - const dbRow = { - id: 'com.acme.published', - manifest: { id: 'com.acme.published', version: '2.0.0' }, - publishedBy: 'u_release', - artifactSize: 4096, - }; - const routes = mount({ list: async () => [dbRow], get: async () => null }, []); - const { body } = await drive(routes, 'GET', PKGS); - - // Fields that are NOT part of the installed-package record still travel. - expect(body.data.packages[0].publishedBy).toBe('u_release'); - expect(body.data.packages[0].artifactSize).toBe(4096); - expect(body.data.packages[0].source).toBe('database'); - }); -}); - -describe('GET /packages/:id — the registry fallback is projected too', () => { - it('serves the entry with the undeclared member dropped', async () => { - const routes = mount(EMPTY_DB, [registryEntry({ liveEngineHandle: cyclicEngine() })]); - const { status, body } = await drive( - routes, 'GET', `${PKGS}/:id`, { params: { id: 'com.example.showcase' } }, - ); - - expect(status).toBe(200); - expect(() => JSON.stringify(body)).not.toThrow(); - expect('liveEngineHandle' in body.data.package).toBe(false); - expect(body.data.package.manifest.id).toBe('com.example.showcase'); - expect(body.data.package._diagnostics).toEqual([{ code: 'SOMETHING', message: 'noted' }]); - expect(body.data.package.source).toBe('registry'); - }); - - it('a genuine MISS is still a 404 with this door’s own wording', async () => { - // The other direction: projection must not turn "absent" into "present". - const routes = mount(EMPTY_DB, [registryEntry()]); - const { status, body } = await drive( - routes, 'GET', `${PKGS}/:id`, { params: { id: 'no.such.package' } }, - ); - - expect(status).toBe(404); - expect(body.error.code).toBe('RESOURCE_NOT_FOUND'); - expect(body.error.message).toBe('Package "no.such.package" was not found.'); - }); -}); diff --git a/packages/rest/src/package-routes-coded-error-mapping.test.ts b/packages/rest/src/package-routes-coded-error-mapping.test.ts index 86d2b1ad37..08a2a2eb8f 100644 --- a/packages/rest/src/package-routes-coded-error-mapping.test.ts +++ b/packages/rest/src/package-routes-coded-error-mapping.test.ts @@ -165,43 +165,18 @@ const SITES: Site[] = [ }, }, { - name: 'GET /packages — the capability gate resolver throws', + name: 'POST /packages/publish — the capability gate resolver throws', run: async (error: unknown) => { const resolveExecutionContext = vi.fn(() => { throw error; }); const captured = await drive( - mount({ list: async () => [] }, { resolveExecutionContext }), - 'GET', - PKGS, + mount({ publish: async () => ({ success: true }) }, { resolveExecutionContext }), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); return { captured, reached: () => resolveExecutionContext.mock.calls.length === 1 }; }, }, - { - name: 'GET /packages/:id — packageService.get throws', - run: async (error: unknown) => { - const get = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ get }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => get.mock.calls.length === 1 }; - }, - }, - { - name: 'DELETE /packages/:id — packageService.delete throws', - run: async (error: unknown) => { - const del = vi.fn(async () => { throw error; }); - const captured = await drive( - mount({ delete: del }), - 'DELETE', - `${PKGS}/:id`, - { params: { id: 'com.acme.crm' } }, - ); - return { captured, reached: () => del.mock.calls.length === 1 }; - }, - }, ]; /** @@ -369,12 +344,12 @@ describe('#8016 — anti-vacuity: these routes answer normally when nothing thro expect(captured.body?.success).toBe(true); }); - it('GET /packages/:id returns 404 RESOURCE_NOT_FOUND for an absent package', async () => { + it('POST /packages/publish returns 404 RESOURCE_NOT_FOUND when no package service is composed (#7563)', async () => { const captured = await drive( - mount({ get: async () => undefined }), - 'GET', - `${PKGS}/:id`, - { params: { id: 'com.acme.nope' } }, + mount(undefined as any), + 'POST', + `${PKGS}/publish`, + { body: { manifest: MANIFEST, metadata: { author: 'acme' } } }, ); expect(captured.status).toBe(404); expect(captured.body?.error?.code).toBe('RESOURCE_NOT_FOUND'); diff --git a/packages/rest/src/package-routes-query-multiplicity.test.ts b/packages/rest/src/package-routes-query-multiplicity.test.ts deleted file mode 100644 index 954e909372..0000000000 --- a/packages/rest/src/package-routes-query-multiplicity.test.ts +++ /dev/null @@ -1,248 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * `?version=` multiplicity on `/api/v1/packages/:id` (#6307). - * - * `IHttpRequest.query` is declared `Record`, so a - * repeated query parameter arrives as an ARRAY. Both handlers used it as a - * string and handed the array straight to `PackageService`, whose parameter is - * `version?: string`. Measured on `origin/main` before the fix: - * - * GET ?version=1.0.0&version=2.0.0 → packageService.get(id, ['1.0.0','2.0.0']) - * DELETE ?version=1.0.0&version=2.0.0 → packageService.delete(id, ['1.0.0','2.0.0']) - * …and `protocol.deletePackage` NOT called, - * answering 200 "Deleted com.acme.crm@1.0.0,2.0.0" - * - * The DELETE line is the sharp one: `if (!version && protocol.deletePackage)` - * gates the FULL uninstall (metadata rows + the durable `sys_packages` record + - * the registered data-plane cleanups, #2747). A truthy `version` skips it, so a - * repeated parameter silently narrowed the operation's SCOPE and still reported - * success. That is a wrong answer on a destructive verb, so the route refuses - * the ambiguity instead of resolving it — see `readSingleQueryValue`. - * - * Observation-class: no user hits this today, because it takes a client that - * repeats the parameter, and the Hono adapter collapses repeats to the first - * value before a handler sees them. The `node:http` adapter does not (measured: - * `NodeHttpServer` hands `['1.0.0','2.0.0']` through over a real socket), which - * is why the consumer has to handle the shape its contract declares rather than - * depend on which server booted. - * - * What these cases pin, in order: the single-value paths behave EXACTLY as - * before (the fix is not allowed to move them), repetition is refused - * identically on both verbs, and the full-uninstall branch is still reached - * when no version is supplied at all. - */ - -import { describe, it, expect } from 'vitest'; -import type { RouteHandler } from '@objectstack/spec/contracts'; -import { registerPackageRoutes } from './package-routes.js'; - -const PKGS = '/api/v1/packages'; -const ID = 'com.acme.crm'; -const MANIFEST = { id: ID, version: '1.0.0' }; - -interface Captured { status: number; body: any } - -/** Records every argument the service/protocol layer is handed. */ -interface Spy { - getVersions: unknown[]; - deleteVersions: unknown[]; - protocolCalls: number; -} - -function harness(options: { protocol?: boolean } = {}) { - const spy: Spy = { getVersions: [], deleteVersions: [], protocolCalls: 0 }; - const svc = { - get: async (_id: string, version?: string) => { - spy.getVersions.push(version); - return { id: ID, manifest: MANIFEST }; - }, - delete: async (_id: string, version?: string) => { - spy.deleteVersions.push(version); - return { success: true }; - }, - }; - const opts = options.protocol - ? { - protocol: { - deletePackage: async () => { - spy.protocolCalls += 1; - // [#9960] `deleted` is part of the verb's declared response — - // the option's own type says so now, so a double that omits it no - // longer compiles. Empty here: these cases count CALLS, not rows. - return { success: true, deletedCount: 3, failedCount: 0, deleted: [], failed: [], cleanups: [] }; - }, - }, - } - : {}; - - const routes = new Map(); - const server = { - get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, - post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, - put: () => {}, - delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); }, - patch: () => {}, - use: () => {}, - listen: async () => {}, - close: async () => {}, - } as any; - // [#7033 / #7023] The package routes now carry an authorization gate that runs - // BEFORE the `?version=` multiplicity check these cases pin. GET is a read - // route and DELETE a write route, so the caller is stubbed to hold BOTH the - // read set (`studio.access` / `setup.access`) and the write key - // (`manage_metadata`) — every case here then reaches the multiplicity rule it - // is named after. The gate itself is pinned in - // `package-envelope.conformance.test.ts`'s `packages authz` describe. - registerPackageRoutes(server, () => svc as any, '/api/v1', { - resolveExecutionContext: async () => ({ - userId: 'u_pkg', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], - }), - ...opts, - }); - - const drive = async (method: 'GET' | 'DELETE', query: Record): Promise => { - const handler = routes.get(`${method}:${PKGS}/:id`); - if (!handler) throw new Error(`no handler for ${method}`); - const captured: Captured = { status: 200, body: undefined }; - const res: any = { - json(d: any) { captured.body = d; }, - send() {}, - status(c: number) { captured.status = c; return res; }, - header() { return res; }, - }; - await handler( - { params: { id: ID }, query, body: undefined, headers: {}, method, path: `${PKGS}/:id` } as any, - res, - ); - return captured; - }; - - return { spy, drive }; -} - -describe('#6307 — a single `?version=` behaves exactly as before', () => { - it('GET with one value passes that STRING through and answers the same body', async () => { - const { spy, drive } = harness(); - const { status, body } = await drive('GET', { version: '1.0.0' }); - expect(spy.getVersions).toEqual(['1.0.0']); - expect(status).toBe(200); - expect(body).toEqual({ - success: true, - data: { package: { id: ID, manifest: MANIFEST, source: 'database' } }, - }); - }); - - it('GET with no version still asks for `latest`', async () => { - const { spy, drive } = harness(); - await drive('GET', {}); - expect(spy.getVersions).toEqual(['latest']); - }); - - it('GET with an EMPTY `?version=` still asks for `latest` (falsy, as before)', async () => { - const { spy, drive } = harness(); - await drive('GET', { version: '' }); - expect(spy.getVersions).toEqual(['latest']); - }); - - it('DELETE with one value stays version-scoped and answers the same body', async () => { - const { spy, drive } = harness({ protocol: true }); - const { status, body } = await drive('DELETE', { version: '1.0.0' }); - expect(spy.deleteVersions).toEqual(['1.0.0']); - expect(spy.protocolCalls).toBe(0); - expect(status).toBe(200); - expect(body).toEqual({ success: true, data: { message: `Deleted ${ID}@1.0.0` } }); - }); -}); - -describe('#6307 — the full-uninstall branch is still reached without a version', () => { - it('DELETE with NO version goes through protocol.deletePackage', async () => { - const { spy, drive } = harness({ protocol: true }); - const { status, body } = await drive('DELETE', {}); - expect(spy.protocolCalls).toBe(1); - expect(spy.deleteVersions).toEqual([]); - expect(status).toBe(200); - expect(body).toEqual({ - success: true, - data: { message: `Deleted ${ID}`, deletedCount: 3, cleanups: [] }, - }); - }); - - it('DELETE with an EMPTY `?version=` still uninstalls fully (falsy, as before)', async () => { - const { spy, drive } = harness({ protocol: true }); - await drive('DELETE', { version: '' }); - expect(spy.protocolCalls).toBe(1); - }); - - it('DELETE with the parameter absent from an EMPTY array is no occurrence at all', async () => { - // A contract-legal encoding of "not supplied". It must not be mistaken for - // a version pin — that would silently narrow the uninstall again. - const { spy, drive } = harness({ protocol: true }); - await drive('DELETE', { version: [] }); - expect(spy.protocolCalls).toBe(1); - }); -}); - -describe('#6307 — one occurrence encoded as a one-element array is still one occurrence', () => { - it('GET accepts `[\'1.0.0\']` and unwraps it', async () => { - const { spy, drive } = harness(); - const { status } = await drive('GET', { version: ['1.0.0'] }); - expect(status).toBe(200); - expect(spy.getVersions).toEqual(['1.0.0']); - }); - - it('DELETE accepts `[\'1.0.0\']` and stays version-scoped', async () => { - const { spy, drive } = harness({ protocol: true }); - const { status } = await drive('DELETE', { version: ['1.0.0'] }); - expect(status).toBe(200); - expect(spy.deleteVersions).toEqual(['1.0.0']); - expect(spy.protocolCalls).toBe(0); - }); -}); - -describe('#6307 — a REPEATED `?version=` is refused, not resolved', () => { - it('GET answers 400 VALIDATION_ERROR and never reaches the service', async () => { - const { spy, drive } = harness(); - const { status, body } = await drive('GET', { version: ['1.0.0', '2.0.0'] }); - expect(status).toBe(400); - expect(body.success).toBe(false); - expect(body.error.code).toBe('VALIDATION_ERROR'); - expect(body.error.message).toContain('"version"'); - expect(body.error.message).toContain('2 times'); - // The array never reaches `version?: string`. - expect(spy.getVersions).toEqual([]); - }); - - it('DELETE answers 400 and performs NO deletion of either kind', async () => { - // The defect answered 200 here, having quietly skipped the full uninstall - // and asked the durable registry to delete "1.0.0,2.0.0". - const { spy, drive } = harness({ protocol: true }); - const { status, body } = await drive('DELETE', { version: ['1.0.0', '2.0.0'] }); - expect(status).toBe(400); - expect(body.error.code).toBe('VALIDATION_ERROR'); - expect(spy.deleteVersions).toEqual([]); - expect(spy.protocolCalls).toBe(0); - }); - - it('both verbs answer the identical body — one rule, one answer', async () => { - const g = await harness().drive('GET', { version: ['a', 'b'] }); - const d = await harness({ protocol: true }).drive('DELETE', { version: ['a', 'b'] }); - expect(g.status).toBe(d.status); - expect(g.body).toEqual(d.body); - }); - - it('two IDENTICAL values are still two occurrences, and still refused', async () => { - // Deliberate: the rule is "supply it at most once", which a client can check - // without knowing our semantics. "at most one DISTINCT value" would be a - // de-duplication rule nobody can predict. - const { spy, drive } = harness({ protocol: true }); - const { status } = await drive('DELETE', { version: ['1.0.0', '1.0.0'] }); - expect(status).toBe(400); - expect(spy.protocolCalls).toBe(0); - }); - - it('three or more occurrences are reported by count', async () => { - const { body } = await harness().drive('GET', { version: ['1', '2', '3'] }); - expect(body.error.message).toContain('3 times'); - }); -}); diff --git a/packages/rest/src/package-routes.ts b/packages/rest/src/package-routes.ts index 38988de559..d15d572e68 100644 --- a/packages/rest/src/package-routes.ts +++ b/packages/rest/src/package-routes.ts @@ -1,12 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { IHttpServer, shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE, rethrowAuthzStoreUnavailable } from '@objectstack/core'; -// [#7020] The read cohort names the READ-ONLY half of the ADR-0106 D4 exemption -// on purpose: `OBJECT_SCHEMA_MASK_EXEMPT_CAPABILITIES` became the derived union -// (write gate ∪ read-only exemptions) under the 2026-08-10 ruling, while this -// gate's cohort was ruled separately (#7033 / #7023) and pins write-only callers -// OUT. Same value it read before — no re-ruling by side effect. -import { OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES } from '@objectstack/metadata-core'; import type { PackageService } from '@objectstack/service-package'; // The declared envelope is written in ONE place for the whole platform (#3973), // and so (#8016) is the rule that reads an HTTP answer off a THROWN error. @@ -23,45 +17,27 @@ import { INTERNAL_ERROR_MESSAGE, } from '@objectstack/types'; import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js'; -import { readSingleQueryValue, repeatedQueryParamMessage } from './query-multiplicity.js'; -// [#9846] The declared meta-read shapes for the `protocol.getMetaItems` seam -// below — imported so this module's idea of the request/response is the SPEC's -// idea of it, not a hand-rolled restatement that the spec can drift away from -// while this file keeps compiling green. Same discipline as the sibling -// meta-read doors in `rest-server.ts` (#9805 / #9741). -import type { GetMetaItemsRequest, GetMetaItemsResponse } from '@objectstack/spec/api'; -// [#9960] The declared uninstall shapes for the `protocol.deletePackage` seam -// below, imported from the PRODUCER for the same reason the meta-read shapes -// above come from the spec: so this module's idea of the request/response is -// the one statement of that contract rather than a local restatement the -// producer can drift away from while this file keeps compiling green. There is -// no spec shape to import for this verb — `deletePackage` is deliberately -// undeclared in `packages/spec` (zero external consumers, #9960) — so the -// producer's own exported type IS the contract. Type-only: no runtime import of -// `@objectstack/metadata-protocol` exists here, and this package still does not -// depend on it at run time (see `query-multiplicity.ts` and `rest-server.ts`, -// which duck-type the same seam for exactly that reason). -import type { DeletePackageRequest, DeletePackageResponse } from '@objectstack/metadata-protocol'; /** * [#7033 / #7023] The authorization gate for the REST package transport. * * `/packages` had TWO HTTP transports and both were ungated: the runtime * dispatcher domain (`packages/runtime/src/domains/packages.ts`) AND this - * `@objectstack/rest` direct-mount registrar — which registers FIRST in the - * production stack (first-match-wins, see the module note above), so for the - * three routes both declare (`GET /packages`, `GET /packages/:id`, - * `DELETE /packages/:id`) THIS transport is the one production actually serves. - * Gating only the dispatcher would leave those routes open — the exact - * one-transport gap #6603/#7019 paid for on `/meta`. - * - * Same ruled policy as the dispatcher (maintainer, 2026-08-09): a domain-wide - * anonymous floor, `manage_metadata` for state-changing routes - * (`POST /packages/publish`, `DELETE /packages/:id`), and the ADR-0106 D4 read - * set (`studio.access` / `setup.access`) for reads (`GET /packages`, - * `GET /packages/:id`). The public MARKETPLACE browse is a different surface - * (`/marketplace/packages`, MarketplaceProxyPlugin) — these `/api/v1/packages` - * routes are management, so denying anonymous here strands no public browse. + * `@objectstack/rest` direct-mount registrar. Gating only the dispatcher would + * have left this registrar's routes open — the exact one-transport gap + * #6603/#7019 paid for on `/meta`. + * + * [#14503] Since the registrar mounts ONE route — `POST /packages/publish`, + * the only verb+path here with no dispatcher twin — the gate has one cohort: + * the same ruled policy as the dispatcher (maintainer, 2026-08-09), a + * domain-wide anonymous floor, then `manage_metadata` for the state-changing + * verb. The read cohort (`studio.access` / `setup.access`, ADR-0106 D4) is + * enforced where the reads are served — the dispatcher domain is the single + * implementation of `GET /packages` and `GET /packages/:id` now, and + * `packages/runtime/src/domains/packages-capability-gate.test.ts` pins that + * cohort. The public MARKETPLACE browse is a different surface + * (`/marketplace/packages`, MarketplaceProxyPlugin) — this `/api/v1/packages` + * route is management, so denying anonymous here strands no public browse. * * The caller context is resolved through {@link PackageRoutesOptions.resolveExecutionContext}, * which the composition wires to the `RestServer`'s own resolver (the SAME @@ -75,13 +51,12 @@ async function refusePackageRequest( options: PackageRoutesOptions, req: any, res: any, - kind: 'read' | 'write', ): Promise { // [#13279] The gate's OWN net. `rethrowAuthzStoreUnavailable` keeps the // fail-closed default for every fault except a permission-store outage, which // must reach `handlePackageRouteError` and be answered as the 503 // `SERVICE_UNAVAILABLE` it is — never as a capability denial the caller could - // mistake for "you lack `studio.access`". + // mistake for "you lack `manage_metadata`". const ctx = options.resolveExecutionContext ? await options.resolveExecutionContext(req).catch(rethrowAuthzStoreUnavailable) : undefined; @@ -99,15 +74,11 @@ async function refusePackageRequest( return true; } const held = new Set(Array.isArray(ctx?.systemPermissions) ? ctx.systemPermissions : []); - const allowed = ctx?.isSystem || (kind === 'write' - ? held.has('manage_metadata') - : OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES.some((c) => held.has(c))); + const allowed = ctx?.isSystem || held.has('manage_metadata'); if (!allowed) { - // Same wrapped envelope, one FORBIDDEN code, message per cohort — the sibling - // `/meta` REST capability gate's shape, built through the shared `sendError`. - sendError(res, 403, 'FORBIDDEN', kind === 'write' - ? 'Managing packages requires the `manage_metadata` capability.' - : 'Reading packages requires the `studio.access` or `setup.access` capability.'); + // Same wrapped envelope, one FORBIDDEN code — the sibling `/meta` REST + // capability gate's shape, built through the shared `sendError`. + sendError(res, 403, 'FORBIDDEN', 'Managing packages requires the `manage_metadata` capability.'); return true; } return false; @@ -130,12 +101,13 @@ async function refusePackageRequest( * retry that cannot succeed, and it hides the one thing the caller needed to * act on (the code). * - * It was also a disagreement rather than merely a bug. The dispatcher twin - * (`packages/runtime/src/domains/packages.ts` → `errorFromThrown`) has always - * read `.status` first and answered 409 for the same throw. Two doors serve - * `/api/v1/packages`; **this** registrar mounts first in the production stack - * (first-match-wins, see the module note above), so the wrong answer was the - * live one. + * It was also a disagreement rather than merely a bug. The dispatcher's + * `/packages` domain (`packages/runtime/src/domains/packages.ts` → + * `errorFromThrown`) has always read `.status` first and answered 409 for the + * same throw, so the two doors that then served `/api/v1/packages` answered + * one refusal two ways. (Since #14503 the read and delete twins are gone from + * this registrar — the dispatcher domain is their single implementation — and + * the rule below governs the one route left, `POST /packages/publish`.) * * ## Why it delegates instead of mapping here * @@ -163,13 +135,14 @@ async function refusePackageRequest( * predates this change and is unchanged by it (filed separately)". Filed as * #8086, and closed here. * - * It was reachable, not theoretical, and was reproduced through this door - * before being fixed — a real `ObjectQL` engine and a real + * It was reachable, not theoretical, and was reproduced through this + * registrar before being fixed — a real `ObjectQL` engine and a real * `ObjectStackProtocolImplementation` whose driver fails the `sys_metadata` - * read the way a missing table does. `DELETE /api/v1/packages/:id` with no - * `?version=` routes to `protocol.deletePackage`, whose FIRST database touch + * read the way a missing table does. The registrar's then-mounted + * `DELETE /api/v1/packages/:id` (removed by #14503) with no `?version=` routed + * to `protocol.deletePackage`, whose FIRST database touch * (`engine.find('sys_metadata', { where })`) sits outside that method's - * per-item `try`, so the driver line propagates whole and arrived here: + * per-item `try`, so the driver line propagated whole and arrived here: * * HTTP 500 * {"success":false,"error":{"code":"INTERNAL_ERROR", @@ -180,9 +153,9 @@ async function refusePackageRequest( * `packages/runtime/src/http-dispatcher.ts`) has run exactly this expression * since #3867, and `rest-server.ts` runs the same predicate at three call * sites. #5437 / PR #5464 closed this class one seam over and never reached - * this registrar, because it does not go through `resolveErrorResponse` at all. - * Two doors serve `/api/v1/packages` and this one mounts FIRST in the - * production stack, so the unfiltered answer was the live one. + * this registrar, because it does not go through `resolveErrorResponse` at all + * — and for `POST /packages/publish` this registrar is the only door, so an + * unfiltered answer here is the live one. * * Scoped to 5xx, deliberately: a 4xx message is a caller-facing answer by * design — the protocol's `[tenant_scope_required]` refusal names the very @@ -199,77 +172,10 @@ async function refusePackageRequest( * the same predicate. Widening it HERE would be a new rule at one door and * would re-create the divergence this closes. The cure is option C — the * producer (`metadata-protocol`) not interpolating driver text into - * client-facing messages at all — which is a separate card. Pinned as a live - * case in `package-door-5xx-message-sanitization.test.ts` so it goes red the - * day either lands. - */ -/** - * The fields a REGISTRY-sourced package entry is declared to carry on this - * door: the installed-package record shape (`InstalledPackageSchema`, - * `@objectstack/spec/kernel` — that schema is the authority; this list mirrors - * it deliberately, so publishing a newly declared field here is a decision - * rather than a side effect), plus `_diagnostics`, which is not part of the - * record at all — `decorateMetadataItem` in `@objectstack/metadata-protocol` - * grafts it onto every item leaving `getMetaItems`, and this door serves that - * output. It is listed because it is MEASURED to be the only thing the - * decoration adds for `type: 'package'`, not because the record declares it. - */ -const REGISTRY_PACKAGE_RESPONSE_FIELDS = [ - 'manifest', - 'status', - 'enabled', - 'installedAt', - 'updatedAt', - 'installedVersion', - 'previousVersion', - 'statusChangedAt', - 'errorMessage', - 'settings', - 'upgradeHistory', - 'registeredNamespaces', - '_diagnostics', - // [#14375] The ADR-0070 D2 writability verdict. Like `_diagnostics` this is - // NOT a record field: `getMetaItems({ type: 'package' })` stamps it on every - // registry item, and this door's contract is to CARRY it (the durable row is - // spread over the registry item, so a durable copy without the key must leave - // the verdict standing) — pinned in `package-list-writable-carry.test.ts`. - // Listed here because an allowlist that omitted it would drop the field - // SILENTLY, answering 200 with the verdict simply gone. - 'writable', -] as const; - -/** - * Project a registry-sourced entry onto its declared fields before it is spread - * into a response — defence in depth behind the `500 Converting circular - * structure to JSON` repair, not the repair itself. - * - * The repair is at the PRODUCER: `SchemaRegistry.installPackage` - * (`@objectstack/objectql`) stores a serializable projection of the manifest - * instead of the caller's live `defineStack()` object, whose `plugins: [...]` - * held initialised plugin instances and through them the engine — a cycle since - * the engine grew `actionActivation -> store -> engine`. So this door has - * nothing unserializable left to hand out. - * - * What the projection adds is the failure MODE for the next undeclared member: - * `{ ...item }` let ONE bad member on ONE package fail the whole list for every - * caller, and an explicit field list degrades the same member to a field this - * response never mentions. Only the REGISTRY half is projected — the database - * half below is `PackageService`'s own durable JSON, whose shape this door does - * not own and must not narrow. - * - * Undefined fields are omitted, so the bytes are unchanged for every entry that - * already served fine. + * client-facing messages at all — which is a separate card. Pinned on the + * publish route in `package-door-5xx-message-sanitization.test.ts` so it goes + * red the day either lands. */ -function toRegistryPackageResponse(item: unknown): Record { - if (item === null || typeof item !== 'object') return {}; - const src = item as Record; - const out: Record = {}; - for (const field of REGISTRY_PACKAGE_RESPONSE_FIELDS) { - if (src[field] !== undefined) out[field] = src[field]; - } - return out; -} - function sendThrownError(res: any, error: unknown): void { const thrown = resolveThrownHttpError(error); // The dispatcher twin's expression, byte for byte — one rule, two doors. @@ -341,16 +247,6 @@ function sendThrownError(res: any, error: unknown): void { ); } -/** - * The `?version=` multiplicity rule (#6307), now shared (#6877). - * - * Both helpers moved to `query-multiplicity.ts` when the same rule was applied - * to `rest-server.ts`'s read points — ONE rule and one refusal message across - * the package, rather than a second implementation free to drift. Behaviour - * here is unchanged; only the definitions' home moved. The module's header - * carries the full argument for why repetition is refused rather than resolved. - */ - /** * Resolve the `package` service AT REQUEST TIME. * @@ -363,8 +259,18 @@ function sendThrownError(res: any, error: unknown): void { * follows registration order for plugins with no edge between them * (`plugin-order.ts`), so on every showcase-shaped deployment the service is * present at request time and absent at the one instant the mount decision was - * taken. Resolving per request makes the answer independent of composition - * order instead of silently encoding it. + * taken. Resolving per request is what lets the HANDLER answer for the + * deployment it is really on. + * + * ⚠️ [#14503] It never made the MOUNT decision independent of composition + * order, whatever this docblock used to claim: `registerPackageRoutes` still + * asked the resolver ONCE, at registration time, to decide whether to mount + * its three service-gated routes — so on that same showcase boot those three + * were never mounted at all (measured: 147 registrations with the service + * absent against 150 with it present; the dispatcher's `/packages` domain + * answered every read and delete). That gate is gone with the routes; the one + * route left mounts unconditionally and this resolver is consulted only per + * request. */ export type PackageServiceResolver = () => PackageService | undefined; @@ -372,45 +278,6 @@ export type PackageServiceResolver = () => PackageService | undefined; * Options for package route registration. */ export interface PackageRoutesOptions { - /** - * Protocol service (ObjectStackProtocol) — provides access to in-memory - * SchemaRegistry packages loaded via defineStack()/AppPlugin at boot time, - * and (#2747) the full `deletePackage` uninstall semantics: package - * metadata rows, the durable `sys_packages` record, and the registered - * data-plane cleanups (e.g. plugin-security revoking the package's - * permission sets and bindings). - */ - protocol?: { - /** - * [#9846] Request/response types are the SPEC's declared shapes, so a - * change to `GetMetaItemsRequest` (a narrowed `type` vocabulary, a newly - * required member, a renamed key) is a compile error HERE instead of a - * silent drift. The member stays OPTIONAL and both call sites keep their - * `typeof … === 'function'` feature-detection: `MetadataProtocol` declares - * this verb REQUIRED, and adopting that whole would change what this seam - * tolerates — a behaviour question, deliberately not answered here. - */ - getMetaItems?(req: GetMetaItemsRequest): Promise; - /** - * [#7780] `allTenants` is the explicit carrier for cross-tenant uninstall - * semantics; the protocol refuses a call that names neither it nor an - * `organizationId` (`TENANT_SCOPE_REQUIRED`, 400). - * - * [#9960] Request/response are the PRODUCER's declared shapes. The local - * restatement they replace named neither `organizationId` nor `keepData` - * and omitted `deleted` from the response — so the one key that decides an - * uninstall's blast radius had no word for it here, while the dispatcher - * twin sent that key on every org-scoped call. The member stays OPTIONAL - * and the call site below keeps its `typeof … === 'function'` - * feature-detection: the `protocol` service slot is deliberately - * uncontracted (`ServiceSlotContracts`), the spec's own `PackageProtocol` - * does not declare this verb at all, and registrants that carry no - * `deletePackage` are real — so requiring the member here would change what - * this seam tolerates, which is a behaviour question this card does not - * answer. - */ - deletePackage?(req: DeletePackageRequest): Promise; - }; /** * [#7033 / #7023] Resolve the caller's execution context for a package route * request. Wired by the composition to the `RestServer`'s own resolver (the @@ -427,103 +294,7 @@ export interface PackageRoutesOptions { } /** - * [#9846] Compile-time pin for the `protocol.getMetaItems` seam above. - * - * WHAT IT CATCHES: that the option's request/response types are still the - * SPEC's declared shapes rather than a hand-rolled restatement of them. The - * defect this card closes is not a wrong call today — both call sites send a - * valid request — it is that a LOCAL structural re-declaration lets the spec - * move underneath this module (a narrowed `type` vocabulary, a newly required - * member, a renamed key) while this file keeps compiling green. A test that - * only drove today's call sites would not notice that; an EXACT type equality - * does, because it fails both when someone re-hand-rolls the local shape and - * when the spec's shape changes without this seam being re-read. - * - * WHY IT LIVES HERE and not in a `*.test.ts`: this package's `tsconfig.json` - * EXCLUDES its `*.test.ts` / `*.spec.ts` files, and no sibling gate - * type-checks them either, so a type-level assertion written in a test file - * would be compiled by nothing — a phantom check that evaluates never and - * stays green when deleted. It sits in compiled source instead, where the - * package's own `typecheck` script (which CI runs) evaluates it. - * - * Mutual assignability would NOT do: the old local `{ type: string }` and - * `GetMetaItemsRequest` are assignable in both directions, so an - * assignability check passes on exactly the shape this card removed. - */ -type ExactlyEqual = - (() => T extends X ? 1 : 2) extends (() => T extends Y ? 1 : 2) ? true : false; - -/** Fails to instantiate unless its argument is exactly `true`. */ -type Pinned = T; - -type DeclaredGetMetaItems = NonNullable['getMetaItems']>; - -/** The REQUEST type is exactly the spec's `GetMetaItemsRequest`. */ -export type _PinGetMetaItemsRequestIsSpecDeclared = Pinned< - ExactlyEqual[0], GetMetaItemsRequest> ->; - -/** The RESPONSE type is exactly the spec's `GetMetaItemsResponse`. */ -export type _PinGetMetaItemsResponseIsSpecDeclared = Pinned< - ExactlyEqual>, GetMetaItemsResponse> ->; - -/** - * The member stays OPTIONAL. `MetadataProtocol` declares `getMetaItems` as a - * REQUIRED member; adopting it whole would change what this seam tolerates - * (both call sites feature-detect with `typeof … === 'function'`), which is a - * behaviour question this card does not answer. This pin fails if a later - * edit quietly makes the member required. - */ -export type _PinGetMetaItemsStaysOptional = Pinned< - undefined extends NonNullable['getMetaItems'] ? true : false ->; - -/** - * [#9960] The same three pins for the `protocol.deletePackage` seam, and for - * the same reason — with one difference worth stating: `getMetaItems` above is - * pinned to the SPEC's declared shapes, while this verb has no spec - * declaration, so the producer (`@objectstack/metadata-protocol`) is the - * contract these pin against. That is the adjudicated shape of #9960, not an - * oversight: declaring a protocol verb for a surface with zero external - * consumers is a spec-seat decision nobody has asked for. - * - * WHAT THEY CATCH: that this option's request/response are still the producer's - * types rather than a hand-rolled restatement of them. Exact equality, not - * mutual assignability — the shape this card removed (`{ packageId; actor?; - * allTenants? }`) is assignable to `DeletePackageRequest` in one direction, so - * an assignability check would have passed on the very divergence that made - * `organizationId` and `keepData` unsayable here. - * - * They live in compiled source, not a `*.test.ts`, for the reason spelled out - * above the `getMetaItems` pins: this package's `tsconfig.json` excludes its - * test files, so a type-level assertion written there is compiled by nothing. - */ -type DeclaredDeletePackage = NonNullable['deletePackage']>; - -/** The REQUEST type is exactly the producer's `DeletePackageRequest`. */ -export type _PinDeletePackageRequestIsProducerDeclared = Pinned< - ExactlyEqual[0], DeletePackageRequest> ->; - -/** The RESPONSE type is exactly the producer's `DeletePackageResponse`. */ -export type _PinDeletePackageResponseIsProducerDeclared = Pinned< - ExactlyEqual>, DeletePackageResponse> ->; - -/** - * The member stays OPTIONAL — see the option's own note. This pin fails if a - * later edit quietly makes it required, which would turn a protocol registrant - * without the verb from a supported shape into a type error. - */ -export type _PinDeletePackageStaysOptional = Pinned< - undefined extends NonNullable['deletePackage'] ? true : false ->; - -/** - * Register package management API routes - * - * Provides endpoints for publishing, retrieving, and managing packages. + * Register the REST package management route. * * Returns the routes it mounted, so the caller can record them on the * `RestServer` that owns the surface (#5822) — the returned array IS the array @@ -531,40 +302,47 @@ export type _PinDeletePackageStaysOptional = Pinned< * * Routes: * - POST /api/v1/packages/publish - Publish a package to the marketplace registry - * - GET /api/v1/packages - List all packages (merges registry + database) - * - GET /api/v1/packages/:id - Get a specific package - * - DELETE /api/v1/packages/:id - Delete a package + * + * ## One route, and why (#14503) + * + * This registrar used to mount three more — `GET /packages`, + * `GET /packages/:id` and `DELETE /packages/:id` — gated on the `package` + * service and documented as SHADOWING the dispatcher's twins at the same + * patterns. Neither half of that sentence held on a stock boot: the gate asked + * `resolvePackageService()` ONCE, here, inside `RestApiPlugin.start()`, and + * `objectstack serve` registers `PackageServicePlugin` AFTER + * `createRestApiPlugin`, so the three were never mounted at all (measured: 147 + * registrations with the service absent against 150 with it present, the + * 3-route delta exactly) and the dispatcher's `/packages` domain answered + * every request — with its own 404 wording (`Package '' not found`) and + * its own envelope (the bare row under `data`, no `{ package }` wrapper and no + * `source` stamp). Two implementations of one URL that had already diverged + * were ruled (maintainer, 2026-09-02, on #14503) to become one: the three + * routes are gone, `packages/runtime/src/domains/packages.ts` is the single + * implementation, and `packages/runtime/src/domains/packages-single-door.test.ts` + * pins the surviving door's wording and envelope so "which door answered" + * stays observable. The REST-only `source: 'registry' | 'database' | 'both'` + * stamp and the `?version=` read (with its repeated-parameter refusal) on + * those verbs went with the routes — recorded in the `@objectstack/rest` + * changeset as deliberately removed, not silently dropped. + * + * `POST /packages/publish` stays because it has NO twin (#7563): nobody else + * serves that verb+path, so when this registrar sat out, the request did not + * 404 — it was absorbed by the dispatcher's `/packages/:id` (with + * `id = "publish"`), and the router answered `405` with + * `Allow: DELETE, GET, HEAD, PATCH`: ANOTHER route's method set, describing + * verbs that would each operate on a package literally named `publish`. "Use + * a different method" is the one answer that misinforms here, because `POST` + * is the only verb this surface ever had. Mounting it always means the path + * has an owner that can tell the truth — the handler when a package service + * is reachable, and an honest 404 naming this surface when none is. * * Marketplace publish lives at `/packages/publish`, NOT at the bare * `POST /packages` (#3610): that verb+path is the dispatcher packages - * domain's *install* route, and this registrar registers first in the - * production stack (first-match-wins), so claiming it here silently - * swallowed every `client.packages.install` call with a 400. The - * dispatcher's own `POST /packages/:id/publish` (ADR-0033 draft publish) - * is two segments — different shape, no clash. - * - * ## Which of these four mount, and why they differ (#7563) - * - * `POST /packages/publish` mounts UNCONDITIONALLY; the other three stay gated - * on the `package` service. That asymmetry is not a compromise — it is the one - * shape that is honest for each: - * - * - The three gated routes have DISPATCHER TWINS at byte-identical patterns - * (`packages/runtime/src/domains/packages.ts` — `GET /packages`, - * `GET /packages/:id`, `DELETE /packages/:id`), mounted unconditionally. - * This registrar shadows them when it runs (first-match-wins). Mounting them - * without a `package` service would replace three WORKING routes with a - * degraded refusal, so absence keeps them where they are. - * - `POST /packages/publish` has NO twin. Nobody else serves that verb+path, - * so when this registrar sits out, the request does not 404 — it is absorbed - * by the dispatcher's `/packages/:id` (with `id = "publish"`), and the - * router answers `405` with `Allow: DELETE, GET, HEAD, PATCH`: ANOTHER - * route's method set, describing verbs that would each operate on a package - * literally named `publish` (#7563). "Use a different method" is the one - * answer that misinforms here, because `POST` is the only verb this surface - * ever had. Mounting it always means the path has an owner that can tell the - * truth — the handler when a package service is reachable, and an honest - * 404 naming this surface when none is. + * domain's *install* route, and a registrar claiming it swallowed every + * `client.packages.install` call with a 400. The dispatcher's own + * `POST /packages/:id/publish` (ADR-0033 draft publish) is two segments — + * different shape, no clash. * * The degraded answer is 404 and not 503: a deployment that composed no * marketplace capability is not going to grow one on retry, and 503 invites @@ -594,22 +372,21 @@ export type _PinDeletePackageStaysOptional = Pinned< * * Generic conditions reuse the STANDARD catalog rather than becoming registered * synonyms of it: a missing request field is `MISSING_REQUIRED_FIELD`, an absent - * package is `RESOURCE_NOT_FOUND`, a request whose own parameters are - * self-contradictory is `VALIDATION_ERROR` (the catalog's generic validation - * failure, and what `HttpStatusErrorCodeMap` already names a bare 400 — see - * `readSingleQueryValue`), an unexpected throw is `INTERNAL_ERROR`. Only - * the package-specific outcomes are registered — `PACKAGE_MANIFEST_INVALID`, - * `PACKAGE_PUBLISH_FAILED`, `PACKAGE_DELETE_PARTIAL`, `PACKAGE_DELETE_FAILED`. + * surface is `RESOURCE_NOT_FOUND`, an unexpected throw is `INTERNAL_ERROR`. + * Only the package-specific outcomes are registered — `PACKAGE_MANIFEST_INVALID` + * and `PACKAGE_PUBLISH_FAILED` on this route (`PACKAGE_DELETE_PARTIAL` and + * `PACKAGE_DELETE_FAILED` belonged to the delete route #14503 removed and stay + * in the ledger only as history). * * [#8016] "An **unexpected** throw is `INTERNAL_ERROR`" is the sentence above, - * and it was right — the CODE had drifted wider than it. Every one of the four + * and it was right — the CODE had drifted wider than it. Every one of the * catch-alls treated *every* throw as unexpected, so a coded, status-carrying * refusal from below (`409 DESTRUCTIVE_CHANGE` out of the metadata protocol, - * reached through `packageService.publish` / `.delete`) was answered as a - * server fault. The word doing the work is "unexpected": a throw that DECLARES - * its own status and a registered code is not unexpected, it is a refusal, and - * it now leaves through {@link sendThrownError} carrying both. `INTERNAL_ERROR` - * is still exactly what an unexpected throw gets — the sentence is unchanged + * reached through `packageService.publish`) was answered as a server fault. + * The word doing the work is "unexpected": a throw that DECLARES its own + * status and a registered code is not unexpected, it is a refusal, and it now + * leaves through {@link sendThrownError} carrying both. `INTERNAL_ERROR` is + * still exactly what an unexpected throw gets — the sentence is unchanged * because it was never the thing that was wrong. */ export function registerPackageRoutes( @@ -621,7 +398,7 @@ export function registerPackageRoutes( const packagesPath = `${basePath}/packages`; /** - * The always-mounted half — see "Which of these four mount" above. + * The one route this registrar mounts — see "One route, and why" above. */ const publishRoute: DirectMountedRoute = // POST /api/v1/packages/publish - Publish a package to the marketplace @@ -631,7 +408,7 @@ export function registerPackageRoutes( metadata: { summary: 'Publish a package to the marketplace registry', tags: ['packages'] }, handler: async (req, res) => { try { - if (await refusePackageRequest(options, req, res, 'write')) return; + if (await refusePackageRequest(options, req, res)) return; // Resolved HERE, not at composition (#7563). Authorization runs first so // an anonymous prober cannot read a deployment's capability composition // off this seam. @@ -715,378 +492,15 @@ export function registerPackageRoutes( }, }; - /** - * The service-gated half — mounted only when a `package` service is - * reachable, because each of these three SHADOWS a live dispatcher twin at - * the same pattern and a degraded shadow is worse than no shadow. - * - * These take the RESOLVED service, not the resolver: the gate below already - * decided on presence, and handing them an optional they would each have to - * re-check would add three branches no deployment can reach. Their bodies are - * unchanged from before #7563. - */ - const serviceGatedRoutes = (packageService: PackageService): readonly DirectMountedRoute[] => [ - // GET /api/v1/packages - List all packages (merges registry + database) - { - method: 'GET', - path: packagesPath, - metadata: { summary: 'List packages (registry + published)', tags: ['packages'] }, - handler: async (_req, res) => { - try { - if (await refusePackageRequest(options, _req, res, 'read')) return; - // Merge two sources: - // 1. Registry packages (in-memory, loaded at boot via defineStack/AppPlugin) - // 2. Database packages (published via POST /packages) - const packagesMap = new Map(); - - // Registry packages (via protocol service → SchemaRegistry). - // - // [#11130] NOT wrapped in a catch, deliberately — this is the OTHER half - // of the two-source merge, and it absorbed a failed registry read into a - // 200 for exactly as long as the durable half did. It used to carry: - // - // } catch { - // // Protocol unavailable — continue with database only - // } - // - // which left nothing on the wire to separate "these are all the packages" - // from "these are the packages I could still see": `total` was reported as - // a complete count either way, and the surviving entries kept - // `source: 'database'`, which reads as PROVENANCE, not as a warning that - // the registry half is missing. Same standing family ruling as the durable - // half below — #10965 · #10677 / PR #10788 · #10789 / PR #10964 · #11063: - // **a read that could not happen must not be reported as a read that found - // nothing.** - // - // ⭐ The PRODUCER already declares its refusal, so this is #11063's edit - // and not a new posture. The live `protocol` service is - // `ObjectStackProtocolImplementation` (`packages/metadata-protocol`), - // whose `getMetaItems` sends every non-benign `sys_metadata` overlay read - // failure through `rethrowUnlessMetadataStoreUnprovisioned` → - // `metadataStoreUnavailableError`: `SERVICE_UNAVAILABLE` / 503 with an - // ADR-0112 status+code ON the error, the same envelope #10965 gave - // `PackageService.list()`. {@link sendThrownError} carries that status and - // code through the declared envelope rather than re-deciding them. The one - // benign reason a registry read can fail — `sys_metadata` not provisioned - // yet — is NOT a throw at all on that path (`isMissingTableError`), so - // first boot still lists the registry set. - // - // The `if` guard above is a DIFFERENT case and is untouched: a composition - // with no protocol service is an absence, not a failed read, and still - // answers 200 with the durable half alone. - // - // ⛔ The card's shape (c) — keep the 200 and make the tolerance visible - // with a partial-result marker — is a response-shape change and therefore - // a contract decision; it was NOT authorized by this card's grading, and - // no wire field is added here. - if (options.protocol && typeof options.protocol.getMetaItems === 'function') { - const result = await options.protocol.getMetaItems({ type: 'package' }); - if (result?.items) { - // [#9846] The declared `GetMetaItemsResponse` types `items` as - // `unknown[]` — the spec says nothing about what a metadata item - // CONTAINS. The registry-specific keys read below (`manifest.id`) - // are not spec-declared, so the ELEMENT read stays runtime-shaped - // on purpose, exactly as the sibling meta-read doors do via - // `metaItemsArray` in `rest-server.ts`. The seam itself is now - // spec-typed; this coercion is confined to the read and changes - // no behaviour (a malformed entry still throws, as it did when the - // local shape claimed `any[]` — since #11130 it reaches the outer - // catch and is answered as the 500 a fault deserves, instead of - // being swallowed into a 200). - for (const item of result.items as any[]) { - const id = item.manifest?.id || item.id; - if (id) { - packagesMap.set(id, { - ...toRegistryPackageResponse(item), - source: 'registry', - }); - } - } - } - } - - // Database packages (published artifacts). - // - // [#11063] NOT wrapped in a catch, deliberately — this is the half that - // used to absorb a failed durable read into a 200. The absorbed failure - // left nothing on the wire to separate "these are all the packages" from - // "these are the packages I could still see": `total` was reported as a - // complete count either way, and the registrar-sourced entries kept - // `source: 'registry'`, which reads as PROVENANCE, not as a warning that - // the database half is missing. A refusal the caller never sees is the - // family this repo has already ruled on — #10965 · #10677 / PR #10788 · - // #10789 / PR #10964: **a read that could not happen must not be reported - // as a read that found nothing.** Here it was one level up, in a - // consumer-side catch rather than in a flattener. - // - // What escapes is exactly ONE throw, and it is a declared refusal, not a - // fault: `PackageService.list()` catches its own driver faults and still - // answers `[]` (logging at error), and re-throws only the #10965 seam - // refusal — `SERVICE_UNAVAILABLE` / 503 with the ADR-0112 status+code on - // the error — raised when the storage seam ACCEPTED the query and - // returned no result set. The outer catch hands it to - // {@link sendThrownError}, which carries the producer's own status and - // code through the declared envelope rather than re-deciding them. - // - // ⭐ This ALIGNS the two read doors rather than inventing a posture: - // `GET /packages/:id` next door has never had an inner catch on its - // DURABLE read, so THAT half has answered this same 503 since #10965. The - // list door answering 200 while the detail door refused was the - // inconsistency, not the fix. - // - // ⚠️ [#11376] The qualifier is load-bearing and this note was written - // without it, as a claim about the whole door. It was false: the detail - // door's REGISTRY read carried its own `catch {}`, and swallowing there - // was worse than here — control fell through to a terminal - // `404 RESOURCE_NOT_FOUND` for a read that could not happen. #11376 - // removed it. Both halves of both read doors now carry the producer's - // refusal, which is what makes the alignment claim above true. - // - // ⛔ The alternative the card sketched — keep the 200 and add a declared - // partial-result marker — is a response-shape change and therefore a - // contract decision; it was NOT authorized by this card's grading, and no - // wire field is added here. - const dbPackages = await packageService.list(); - for (const pkg of dbPackages) { - const id = pkg.manifest?.id || pkg.id; - if (id) { - // Database entry takes precedence (has richer metadata from publish) - packagesMap.set(id, { - ...packagesMap.get(id), - ...pkg, - source: packagesMap.has(id) ? 'both' : 'database', - }); - } - } - - const packages = Array.from(packagesMap.values()); - sendOk(res, { packages, total: packages.length }); - } catch (error) { - sendThrownError(res, error); - } - }, - }, - - // GET /api/v1/packages/:id - Get a specific package - { - method: 'GET', - path: `${packagesPath}/:id`, - metadata: { summary: 'Get a package by id', tags: ['packages'] }, - handler: async (req, res) => { - try { - if (await refusePackageRequest(options, req, res, 'read')) return; - const packageId = req.params.id; - const requested = readSingleQueryValue(req.query?.version); - if (!requested.ok) { - sendError(res, 400, 'VALIDATION_ERROR', repeatedQueryParamMessage('version', requested.count)); - return; - } - const version = requested.value || 'latest'; - - // Try database first (richer data from publish) - const pkg = await packageService.get(packageId, version); - if (pkg) { - sendOk(res, { package: { ...pkg, source: 'database' } }); - return; - } - - // Fall back to registry (in-memory loaded packages). - // - // [#11376] NOT wrapped in a catch, deliberately. This read used to carry - // its own inner catch: - // - // } catch { - // // Protocol unavailable - // } - // - // and it is the WORSE half of this family, not a smaller one. Control - // fell straight through to the `sendError` below, so a registry read that - // COULD NOT HAPPEN was answered as `404 RESOURCE_NOT_FOUND` — - // `Package "" was not found.` The list door's version of the same - // swallow (#11130) at least answered a 200 whose `total` merely - // UNDER-COUNTED; this one answers a TERMINAL NEGATIVE FACT, and a caller - // acts on it: an installer decides the package is not installed and - // offers to install it, a console hides the entry, a script branches to - // the create path. The producer's own words for this condition are the - // opposite — *"whether this item exists is unknown"*. - // - // Same standing family ruling as every sibling — #10965 · #10677 / - // PR #10788 · #10789 / PR #10964 · #11063 · #11130: **a read that could - // not happen must not be reported as a read that found nothing.** - // - // ⭐ It is #5532's defect resurfacing one layer up, which is why removing - // the catch is the whole repair. `ObjectStackProtocolImplementation` - // (`packages/metadata-protocol`) — the live `protocol` service this - // registrar is handed — was taught by #5532 NOT to report an unreadable - // `sys_metadata` as "that item does not exist"; this consumer-side catch - // then re-applied precisely that relabelling to the protocol's answer. - // The PRODUCER therefore already declares the refusal: every non-benign - // `sys_metadata` overlay read failure leaves `getMetaItems` through - // `rethrowUnlessMetadataStoreUnprovisioned` → `metadataStoreUnavailableError`, - // i.e. `SERVICE_UNAVAILABLE` / 503 with an ADR-0112 status+code ON the - // error — the same envelope #10965 gave `PackageService.get()` one line - // above. {@link sendThrownError} carries that status and code through the - // declared envelope rather than re-deciding them. The one benign reason a - // registry read can fail — `sys_metadata` not provisioned yet — is NOT a - // throw at all on that path (`isMissingTableError`), so first boot still - // resolves a registry hit. - // - // ⛔ What did NOT move. The defect is that a failed read was - // INDISTINGUISHABLE from an absent resource, so the repair has to keep - // the other direction intact, and both are pinned in - // `package-id-registry-read-refusal.test.ts`: - // - a genuine MISS — both sources read fine and neither holds the id — - // is still `404 RESOURCE_NOT_FOUND`, unchanged; - // - a composition with NO protocol service is an absence, not a failed - // read; the `if` guard below is untouched and that deployment still - // reaches the same 404. - // - // ⛔ No wire field is added: the response shape is a contract decision - // and this card does not carry one. - if (options.protocol && typeof options.protocol.getMetaItems === 'function') { - const result = await options.protocol.getMetaItems({ type: 'package' }); - const match = result?.items?.find((item: any) => - (item.manifest?.id || item.id) === packageId - ); - if (match) { - sendOk(res, { package: { ...toRegistryPackageResponse(match), source: 'registry' } }); - return; - } - } - - sendError(res, 404, 'RESOURCE_NOT_FOUND', `Package "${packageId}" was not found.`); - } catch (error) { - sendThrownError(res, error); - } - }, - }, - - // DELETE /api/v1/packages/:id - Delete a package - { - method: 'DELETE', - path: `${packagesPath}/:id`, - metadata: { summary: 'Delete a package', tags: ['packages'] }, - handler: async (req, res) => { - try { - if (await refusePackageRequest(options, req, res, 'write')) return; - const packageId = req.params.id; - // Refused BEFORE the branch below, because the branch below is exactly - // what a repeated `?version=` silently changed (#6307): the truthiness of - // `version` is what decides full uninstall vs version-scoped delete. - const requested = readSingleQueryValue(req.query?.version); - if (!requested.ok) { - sendError(res, 400, 'VALIDATION_ERROR', repeatedQueryParamMessage('version', requested.count)); - return; - } - const version = requested.value; - - // [#2747] A FULL uninstall (no version pin) goes through - // protocol.deletePackage — one uninstall semantic, not three dialects: - // it removes the package's metadata rows, drops the durable - // sys_packages record, and runs the registered data-plane cleanups - // (plugin-security revokes the package's permission sets/bindings — - // no ghost grants). A version-scoped delete keeps the narrow durable - // registry semantics, as does a deployment without the protocol. - if (!version && typeof options.protocol?.deletePackage === 'function') { - // [#7780] `allTenants: true` is stated, not implied. This registrar has - // no organization to resolve — `packages/rest` carries no - // `resolveActiveOrganizationId` and no org plumbing at all (the - // dispatcher twin owns that seam), so of the two doors the ruling - // allows — resolve an org, or declare the cross-tenant intent — only - // the second is available here. - // - // This preserves the behaviour this door has always had (a full - // uninstall through it is package-wide, which #7705 case 4 pinned on - // purpose); what changes is that the width is now DECLARED at the call - // site instead of being inferred from an argument nobody passed. The - // protocol now refuses the undeclared form outright, so the two doors - // can no longer disagree by accident. - const result = await options.protocol.deletePackage({ packageId, allTenants: true }); - // Zero metadata rows is still a successful uninstall (e.g. a - // runtime-registered package that never published metadata) — - // only per-item failures make it a failure. - if (result.failedCount === 0) { - sendOk(res, { - message: `Deleted ${packageId}`, - deletedCount: result.deletedCount, - cleanups: result.cleanups, - }); - return; - } - // Was a bare `{ success: false, failed, cleanups }` — a failure with no - // `error` at all, so a caller learned that it failed but never why. The - // per-item detail is preserved under the declared `error.details`. - sendError( - res, - 400, - 'PACKAGE_DELETE_PARTIAL', - `Deleting ${packageId} left ${result.failedCount} item(s) behind.`, - { details: { failed: result.failed, cleanups: result.cleanups } }, - ); - return; - } - - const result = await packageService.delete(packageId, version); - - if (result.success) { - sendOk(res, { - message: `Deleted ${packageId}${version ? `@${version}` : ''}`, - }); - return; - } - - // [#8275] A REPORTED delete failure is a DRIVER FAULT, and a driver fault - // is a **5xx**. The statement that failed is `DELETE FROM sys_packages - // WHERE id = ? [AND version = ?]`; a missing table, a lock timeout or a - // foreign-key restriction there is a SERVER fault, and answering `400` - // invited the caller to fix a request that was never the problem while - // hiding a real fault from every dashboard that buckets by status. The - // sibling of what #8131 fixed for `publish` and #8016 for the throw path. - // - // The CALLER's own errors on this route are unaffected and still 4xx: the - // repeated-`?version=` refusal above is checked before `delete` is called - // at all, `PACKAGE_DELETE_PARTIAL` keeps its 400 (per-item uninstall - // failures are a different outcome, not this one), and a coded refusal - // thrown from below `delete` is re-thrown by the producer and answered by - // {@link sendThrownError} with its own status — so a `409 - // DESTRUCTIVE_CHANGE` is still a 409, not swept in here. - // - // The code stays `PACKAGE_DELETE_FAILED` rather than becoming - // `INTERNAL_ERROR`: it is registered, it says more than the generic - // fallback, and it discloses nothing. `envelopeViolations` imposes no - // code↔status agreement, so a registered code on a 5xx is conformant. - // - // Unlike `publish`, the MESSAGE needed no fixing and gets none: it is - // built here from the request's own `:id` and `?version=`, so it echoes - // only what the caller sent and has never carried driver text. The - // producer returns a bare flag with no message channel at all - // (`PackageDeleteResult`), which is what keeps that true — this route is - // a status-classification defect only, never a disclosure. - sendError( - res, - 500, - 'PACKAGE_DELETE_FAILED', - `Failed to delete ${packageId}${version ? `@${version}` : ''}.`, - ); - } catch (error) { - sendThrownError(res, error); - } - }, - }, - ]; - /** * ONE declaration of this registrar's surface (#5822): the array below is * what gets mounted on the host server AND what is handed back as the * description of what was mounted. There is no second table to keep in sync — - * see `direct-mount.ts` for why that identity is the whole point. The gate is - * inside the declaration rather than around the call, so "what was mounted" - * stays the array that mounted it on both branches. + * see `direct-mount.ts` for why that identity is the whole point. No service + * gate sits around it any more (#14503): the resolver is a per-request + * concern of the handler, never a mount-time verdict. */ - const packageService = resolvePackageService(); - const routes: readonly DirectMountedRoute[] = packageService - ? [publishRoute, ...serviceGatedRoutes(packageService)] - : [publishRoute]; + const routes: readonly DirectMountedRoute[] = [publishRoute]; return mountDirectRoutes(server, routes); } diff --git a/packages/rest/src/query-multiplicity.ts b/packages/rest/src/query-multiplicity.ts index fbfeab4cec..875e3238bf 100644 --- a/packages/rest/src/query-multiplicity.ts +++ b/packages/rest/src/query-multiplicity.ts @@ -9,10 +9,14 @@ import { RPC_QUERY_ALIAS_SLOTS } from '@objectstack/spec/data'; * (`packages/spec/src/contracts/http-server.ts`). A repeated parameter * (`?package=a&package=b`) is the ARRAY arm, and that arm is not hypothetical: * the `node:http` adapter (`@objectstack/http-conformance`'s `NodeHttpServer`) - * hands it through as `['a','b']`, measured over a real socket on #6878. The - * production Hono adapter happens to collapse repeats to the first value before - * a handler runs — which is why this class is dormant today, and why it stops - * being dormant the moment that collapse is removed (#6878's ruled route 2). + * hands it through as `['a','b']`, measured over a real socket on #6878 — and + * so does the production Hono adapter: `plugin-hono-server`'s `readQuery` has + * kept repeats as an array since #6878 / PR #6941 (that card's ruled route 2), + * pinned green by `packages/qa/http-conformance/src/query-multiplicity.conformance.test.ts` + * ("hands a consumer the SAME operand on either adapter — the ambiguity is + * visible, not collapsed"). This class is LIVE on every adapter the repo + * ships; an earlier version of this header called it dormant behind a Hono + * collapse that no longer exists (#14503 retired the sentence). * * ## Why the answer is a refusal and not a rule for picking * @@ -51,9 +55,12 @@ import { RPC_QUERY_ALIAS_SLOTS } from '@objectstack/spec/data'; * site names the parameters it declares single-valued instead of gating "every * key in `req.query`". * - * #6307 landed the first copy of this rule in `package-routes.ts`. The two pure - * helpers now live here so there is ONE rule and one message, not a second - * implementation that drifts. + * #6307 landed the first copy of this rule in `package-routes.ts`, on the + * `?version=` of that registrar's package read/delete routes. Those routes are + * gone (#14503 — the dispatcher's `/packages` domain is their single + * implementation, and it reads no `version`), so the rule now has one home: + * here, for the `rest-server.ts` read points — ONE rule and one message, not + * a second implementation that drifts. */ /** diff --git a/packages/rest/src/rest-api-plugin-objectql-provider-three-state.test.ts b/packages/rest/src/rest-api-plugin-objectql-provider-three-state.test.ts index 7da3853ccc..578ca5f872 100644 --- a/packages/rest/src/rest-api-plugin-objectql-provider-three-state.test.ts +++ b/packages/rest/src/rest-api-plugin-objectql-provider-three-state.test.ts @@ -296,9 +296,13 @@ function mountDoor(rest: InstanceType): Map { routes.set(`DELETE:${p}`, h); }, patch: () => {}, use: () => {}, listen: async () => {}, close: async () => {}, } as never; + // [#14503] The door is `POST /packages/publish` — the one route the + // registrar mounts now (the read route this file used to drive is the + // dispatcher domain's alone). Same resolver, same gate, same three wire + // answers: the fixture's permission set grants `manage_metadata`. registerPackageRoutes( server, - () => ({ list: async () => [], publish: async () => ({}), delete: async () => ({}) }) as never, + () => ({ publish: async () => ({ success: true }) }) as never, '/api/v1', { resolveExecutionContext: (req: unknown) => rest.resolvePackageRouteExecutionContext(req) } as never, ); @@ -306,8 +310,8 @@ function mountDoor(rest: InstanceType): Map): Promise<{ status: number; body: any }> { - const handler = routes.get('GET:/api/v1/packages'); - if (!handler) throw new Error('no handler for GET /api/v1/packages'); + const handler = routes.get('POST:/api/v1/packages/publish'); + if (!handler) throw new Error('no handler for POST /api/v1/packages/publish'); const capturedRes: { status: number; body: any } = { status: 0, body: undefined }; const res: any = { json(data: unknown) { capturedRes.body = data; }, @@ -315,7 +319,10 @@ async function driveDoor(routes: Map): Promise<{ status: n status(code: number) { capturedRes.status = code; return res; }, header() { return res; }, }; - await handler({ params: {}, query: {}, body: undefined, headers: {}, method: 'GET', path: '/api/v1/packages' } as never, res); + await handler({ + params: {}, query: {}, headers: {}, method: 'POST', path: '/api/v1/packages/publish', + body: { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }, + } as never, res); return capturedRes; } diff --git a/packages/rest/src/rest-api-plugin-slot-lookups.test.ts b/packages/rest/src/rest-api-plugin-slot-lookups.test.ts index 5a0321aa0d..05e1002ca9 100644 --- a/packages/rest/src/rest-api-plugin-slot-lookups.test.ts +++ b/packages/rest/src/rest-api-plugin-slot-lookups.test.ts @@ -97,7 +97,11 @@ const BOOT_SLOTS = [ 'kernel-manager', 'env-registry', 'kernel-resolver', - 'package', + // [#14503] `package` is NOT here any more: the package registrar mounts + // `POST /packages/publish` unconditionally and resolves the `package` slot + // per request, inside the handler (#7563) — the boot-time lookup that used + // to decide whether to mount its three (since-removed) read/delete routes + // is gone with them. Same shape as `external-datasource` below. ] as const; function mockServer() { diff --git a/packages/rest/src/rest-api-plugin.ts b/packages/rest/src/rest-api-plugin.ts index 701844b5cf..6e67a2e27a 100644 --- a/packages/rest/src/rest-api-plugin.ts +++ b/packages/rest/src/rest-api-plugin.ts @@ -574,7 +574,6 @@ export function createRestApiPlugin(config: RestApiPluginConfig = {}): Plugin { recorder: restServer, ctx, versionedBase, - protocol, // [#7033 / #7023] The package routes' authorization gate reads // the SAME identity resolution the rest of the surface does. resolveExecutionContext: (req) => restServer.resolvePackageRouteExecutionContext(req), diff --git a/packages/rest/src/rest-route-ledger.ts b/packages/rest/src/rest-route-ledger.ts index e17e958aa6..319c58aca1 100644 --- a/packages/rest/src/rest-route-ledger.ts +++ b/packages/rest/src/rest-route-ledger.ts @@ -444,15 +444,14 @@ export const REST_ROUTE_LEDGER: readonly RestRouteLedgerEntry[] = [ { route: 'POST /api/v1/data/:object/updateMany', family: 'batch', source: 'route-manager', disposition: 'sdk', client: 'data.updateMany' }, { route: 'POST /api/v1/data/:object/deleteMany', family: 'batch', source: 'route-manager', disposition: 'sdk', client: 'data.deleteMany' }, - // ── packages (direct-mount registrar; the three `:id` rows service-gated) ── + // ── packages (direct-mount registrar) ────────────────────────────────────── + // ONE row since #14503: the three read/delete twins the registrar used to + // mount beside `publish` are gone, and `packages/runtime`'s `/packages` + // domain is the single implementation of `GET /packages`, + // `GET /packages/:id` and `DELETE /packages/:id` — their rows live in + // `packages/runtime/src/route-ledger.ts`. { route: 'POST /api/v1/packages/publish', family: 'packages', source: 'direct-mount', disposition: 'server-only', note: 'marketplace registry publish ({manifest, metadata}) — publisher tooling, not app-SDK surface. Moved off the bare POST /packages in #3610: that verb+path is the dispatcher install route, and REST registering it first swallowed every packages.install call with a 400. Mounted UNCONDITIONALLY since #7563 — it has no dispatcher twin, so while it was service-gated the path was absorbed by /packages/:id and answered 405 with THAT route\'s Allow set; it now resolves the `package` service per request and answers an honest 404 on a deployment that composes none.' }, - { route: 'GET /api/v1/packages', family: 'packages', source: 'direct-mount', disposition: 'sdk', client: 'packages.list', - note: 'shadows the dispatcher twin (registered first); merges registry + database packages' }, - { route: 'GET /api/v1/packages/:id', family: 'packages', source: 'direct-mount', disposition: 'sdk', client: 'packages.get', - note: 'shadows the dispatcher twin (registered first)' }, - { route: 'DELETE /api/v1/packages/:id', family: 'packages', source: 'direct-mount', disposition: 'sdk', client: 'packages.uninstall', - note: 'shadows the dispatcher twin (registered first); full uninstall via protocol.deletePackage (#2747)' }, // ── external datasource federation (ADR-0015 §6.2, direct-mount) ────────── { route: 'GET /api/v1/datasources/:name/external/tables', family: 'external-datasource', source: 'direct-mount', disposition: 'sdk', client: 'datasources.external.listTables' }, diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index e1276d369c..b2c85f4adb 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -4513,12 +4513,21 @@ export class RestServer { // That move landed with NO edit in this block, which is // exactly the property #6633 was built to provide. // - // A boot that mounted nothing (no `package` service ⇒ - // the registrar was never called) advertises nothing: - // the protocol's service-presence `packages` entry is + // A boot that mounted nothing advertises nothing: the + // protocol's service-presence `packages` entry is // deleted rather than left to promise a 404 — this // server knows the mount fact, which is strictly better - // knowledge than service presence. + // knowledge than service presence. [#14503] The package + // registrar's ONE route (`POST {base}/packages/publish`) + // mounts on every boot since #7563, so `routes.packages` + // is advertised on every boot at THIS server's base; the + // family's reads and delete are served by the runtime + // dispatcher's `/packages` domain, the single + // implementation. (While the base was keyed on the + // registrar's own `GET {base}/packages` copy — never + // mounted on a stock boot, where the `package` service + // registers after this plugin starts — a stock boot + // advertised no `routes.packages` at all.) const direct = this.getDirectMountRouteBases( isScoped ? (req.params?.environmentId ?? ':environmentId') : undefined, ); @@ -13430,11 +13439,20 @@ export class RestServer { let packagesScoped: string | undefined; let datasources: string | undefined; for (const { method, path } of this.directMountedRoutes) { - // The package registrar's list route (`GET {base}/packages`) IS the - // surface base — recorded verbatim, recognised, never rebuilt. - if (method === 'GET' && path.endsWith('/packages')) { - if (path.includes(SCOPED_SEGMENT)) packagesScoped = path; - else packagesUnscoped = path; + // [#14503] The package registrar mounts ONE route, + // `POST {base}/packages/publish`, under the family base; the base + // is that recorded path minus its `/publish` segment — recognised, + // never rebuilt. (It used to be keyed on the registrar's own + // `GET {base}/packages` copy of the list route, removed by #14503: + // the dispatcher's `/packages` domain is the family's single + // implementation, and REST's contribution to the family is publish.) + const publishAt = path.endsWith('/packages/publish') && method === 'POST' + ? path.length - '/publish'.length + : -1; + if (publishAt > 0) { + const base = path.slice(0, publishAt); + if (path.includes(SCOPED_SEGMENT)) packagesScoped = base; + else packagesUnscoped = base; } // Every federation route sits under // `{base}/datasources/:name/external/…`; the advertised base is diff --git a/packages/runtime/src/domains/packages-single-door.test.ts b/packages/runtime/src/domains/packages-single-door.test.ts new file mode 100644 index 0000000000..5a791550c3 --- /dev/null +++ b/packages/runtime/src/domains/packages-single-door.test.ts @@ -0,0 +1,220 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `/packages` has ONE implementation, and this file keeps "which door + * answered" observable (#14503). + * + * ## What was measured + * + * `GET /api/v1/packages` and `GET /api/v1/packages/:id` used to have two HTTP + * implementations: this domain, and three service-gated routes in + * `@objectstack/rest`'s `registerPackageRoutes` that documented themselves as + * SHADOWING this one. On a stock showcase boot the dispatcher answered — the + * 404 wording decided it, `Package 'x' not found` (this file) against + * `Package "x" was not found.` (the REST twin) — because the REST gate asked + * for the `package` service once, at registration time, before + * `PackageServicePlugin` had registered, so the three were never mounted at + * all. The two bodies had already diverged (a `{ package }` wrapper and a + * `source: 'registry' | 'database' | 'both'` stamp on one side, the bare row + * on the other). The maintainer ruled (2026-09-02) that the REST three are + * removed and this domain is the single implementation. + * + * ## What this file pins + * + * The surviving door's WORDING and ENVELOPE, unscoped and environment-scoped: + * + * - a missing package answers `404 RESOURCE_NOT_FOUND` with the message + * `Package '' not found` — single quotes, no trailing period — and + * never the retired REST spelling; + * - a found package answers `{ success: true, data: }`: the bare + * installed-package row under `data`, with no `package` wrapper and no + * `source` key; + * - the list answers `{ packages, total }` whose rows carry no `source`; + * - the same answers arrive through the environment-scoped URL + * (`/environments/:environmentId/packages…`), because since #15859 the + * `@objectstack/hono` catch-all's scoped path is stripped to the domain's + * shape before `DomainHandlerRegistry` resolves it — which is what makes + * this domain the single implementation on the scoped mount too, where + * the REST registrar's mirror used to be the only door. + * + * ## Why this suite drives `dispatch()` with a hand-derived subpath + * + * `packages/runtime` cannot depend on `@objectstack/hono` (that adapter + * depends on THIS package), and the adapter's own suite aliases this package + * to a mock. The adapter's one contribution is + * `const subPath = c.req.path.substring(prefix.length)` in its + * `app.all(`${prefix}/*`)` catch-all, reproduced here exactly, as + * `http-dispatcher.scoped-url-strip.test.ts` does. The same requests were + * driven through the REAL adapter on the built dist while this was written + * (quoted on PR #14503's body); this file is the durable pin. + * + * Identity goes through the real resolver: the kernel offers an `auth` + * session and an ObjectQL engine whose `find` answers the permission-set + * tables the shared authz resolver reads, so the caller holds + * `studio.access` + `manage_metadata` the way a real one would, and the gate + * inside the domain is exercised rather than bypassed. + */ + +import { describe, it, expect } from 'vitest'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { HttpDispatcher, type HttpDispatcherResult } from '../http-dispatcher.js'; + +const PREFIX = '/api/v1'; +const ENV_ID = 'env_alpha'; +const PKG_ID = 'com.acme.crm'; +const MISSING = 'no.such.package'; + +/** The retired REST door's spelling — pinned ABSENT so a resurrection is a red test. */ +const REST_SPELLING = `Package "${MISSING}" was not found.`; +/** This door's spelling, verbatim. */ +const DISPATCHER_SPELLING = `Package '${MISSING}' not found`; + +/** The `@objectstack/hono` catch-all's one contribution, reproduced exactly. */ +function subPathAsTheCatchAllDerivesIt(url: string): string { + return url.substring(PREFIX.length); +} + +/** + * The fixture's ONE hand-written where-matcher: equality plus `$in` — the two + * shapes the shared resolver actually issues — and it REFUSES every other + * shape loudly instead of silently matching (the check:where-matcher + * convention). + */ +function matchesWhere(row: any, where: any): boolean { + for (const [field, cond] of Object.entries(where ?? {})) { + if (field.startsWith('$')) { + throw new Error(`fixture where-matcher: unsupported combinator '${field}'`); + } + if (cond !== null && typeof cond === 'object') { + const ops = Object.keys(cond as object); + if (ops.length !== 1 || ops[0] !== '$in' || !Array.isArray((cond as any).$in)) { + throw new Error(`fixture where-matcher: unsupported operator shape on '${field}'`); + } + if (!(cond as any).$in.includes(row[field])) return false; + continue; + } + if (row[field] !== cond) return false; + } + return true; +} + +/** The permission store the shared authz resolver reads, in its shipped shapes. */ +const TABLES: Record = { + sys_user: [{ id: 'u_admin', email: 'u_admin@example.com' }], + sys_user_permission_set: [{ user_id: 'u_admin', permission_set_id: 'ps_pkg' }], + sys_permission_set: [ + { id: 'ps_pkg', name: 'pkg_admin', system_permissions: ['manage_metadata', 'studio.access'] }, + ], +}; + +function registryWith(): SchemaRegistry { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + (registry as any).logLevel = 'silent'; + registry.installPackage({ + id: PKG_ID, + name: 'CRM', + namespace: 'crm', + version: '1.0.0', + type: 'app', + scope: 'user', + objects: [{ name: 'lead', fields: { title: { type: 'text' } } }], + } as any); + return registry; +} + +function kernelWith(registry: SchemaRegistry): any { + const ql = { + registry, + find: async (object: string, q: any = {}) => { + const rows = (TABLES[object] ?? []).filter((row: any) => matchesWhere(row, q?.where)); + return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows; + }, + }; + const auth = { api: { getSession: async () => ({ user: { id: 'u_admin' } }) } }; + const services: Record = { objectql: ql, auth }; + return { + getState: () => 'running', + getService: (n: string) => services[n], + getServiceAsync: async (n: string) => services[n], + }; +} + +/** Exactly how `createHonoApp` builds its dispatcher: `new HttpDispatcher(kernel)`. */ +function dispatcher(): HttpDispatcher { + return new HttpDispatcher(kernelWith(registryWith())); +} + +function responseOf(res: HttpDispatcherResult, what: string): NonNullable { + const { response } = res; + if (!response) throw new Error(`${what} answered no response at all`); + return response; +} + +async function send(method: string, url: string): Promise<{ status: number; body: any; ctx: any }> { + const ctx: any = { request: new Request(`http://pin.local${url}`, { method }) }; + const res = await dispatcher().dispatch(method, subPathAsTheCatchAllDerivesIt(url), undefined, {}, ctx, PREFIX); + const response = responseOf(res, `${method} ${url}`); + return { status: response.status, body: response.body, ctx }; +} + +const UNSCOPED = `${PREFIX}/packages`; +const SCOPED = `${PREFIX}/environments/${ENV_ID}/packages`; + +describe('/packages — one implementation, and its 404 wording says which (#14503)', () => { + it('NEGATIVE CONTROL: a path no domain claims answers 404 ROUTE_NOT_FOUND — the shape "no door answered" takes', async () => { + for (const url of [`${PREFIX}/no-such-domain`, `${PREFIX}/environments/${ENV_ID}/no-such-domain`]) { + const r = await send('GET', url); + expect(r.status).toBe(404); + expect(r.body?.error?.code).toBe('ROUTE_NOT_FOUND'); + } + }); + + for (const [label, base] of [['unscoped', UNSCOPED], ['environment-scoped', SCOPED]] as const) { + it(`${label} GET /packages/:id for a missing package answers the dispatcher's spelling, never the retired REST one`, async () => { + const r = await send('GET', `${base}/${MISSING}`); + expect(r.status).toBe(404); + expect(r.body?.success).toBe(false); + expect(r.body?.error?.code).toBe('RESOURCE_NOT_FOUND'); + expect(r.body?.error?.message).toBe(DISPATCHER_SPELLING); + expect(r.body?.error?.message).not.toBe(REST_SPELLING); + expect(r.body?.error?.message).not.toContain('was not found'); + }); + + it(`${label} DELETE /packages/:id for a missing package answers the same spelling`, async () => { + const r = await send('DELETE', `${base}/${MISSING}`); + expect(r.status).toBe(404); + expect(r.body?.error?.code).toBe('RESOURCE_NOT_FOUND'); + expect(r.body?.error?.message).toBe(DISPATCHER_SPELLING); + }); + + it(`${label} GET /packages/:id for an installed package answers the BARE row under data — no { package } wrapper, no source stamp`, async () => { + const r = await send('GET', `${base}/${PKG_ID}`); + expect(r.status).toBe(200); + expect(r.body?.success).toBe(true); + expect(r.body?.data?.manifest?.id).toBe(PKG_ID); + expect(r.body?.data?.package).toBeUndefined(); + expect('source' in (r.body?.data ?? {})).toBe(false); + // The dispatcher envelope, measured: `data` + `meta` beside the flag. + // The retired REST door answered `{ data, success }` with the row + // one level down under `data.package` — the key set is part of + // "which door answered". + expect(Object.keys(r.body).sort()).toEqual(['data', 'meta', 'success']); + }); + + it(`${label} GET /packages answers { packages, total } and its rows carry no source stamp`, async () => { + const r = await send('GET', base); + expect(r.status).toBe(200); + expect(r.body?.success).toBe(true); + expect(r.body?.data?.total).toBe(1); + expect(r.body?.data?.packages).toHaveLength(1); + expect(r.body?.data?.packages[0]?.manifest?.id).toBe(PKG_ID); + expect('source' in r.body.data.packages[0]).toBe(false); + }); + } + + it('the scoped URL names its environment on the SAME request the domain served — one convention, read once', async () => { + const r = await send('GET', `${SCOPED}/${PKG_ID}`); + expect(r.status).toBe(200); + expect(r.ctx.urlEnvironmentId).toBe(ENV_ID); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 41b4e33082..cf8ed623be 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -28,7 +28,7 @@ import { // exemptions) when the maintainer ruled the two sets must not be hand-kept // separately. This gate wants the read-only half specifically: its cohort was // ruled on its own terms (#7033 / #7023) and pinned WRITE-only callers OUT -// (`packages/rest/src/package-envelope.conformance.test.ts`), so it names +// (`packages-capability-gate.test.ts`, beside this file), so it names // `OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES` — same value it read before, // no re-ruling of the package cohort as a side effect of #7020. import { OBJECT_SCHEMA_READ_ONLY_EXEMPT_CAPABILITIES } from '@objectstack/metadata-core'; @@ -41,7 +41,8 @@ import { isWritablePackage } from '@objectstack/metadata-protocol'; // [#9960] The uninstall seam's DECLARED shapes, from the same producer and for // the same reason as the predicate above: this door reached `deletePackage` // through `protocol` and routinely sent two keys — `organizationId` -// and `keepData` — that the sibling REST door's own option type could not even +// and `keepData` — that the since-removed REST twin's own option type (#14503) +// could not even // express. One statement of the contract, imported by both doors. import type { DeletePackageRequest, DeletePackageResponse } from '@objectstack/metadata-protocol'; // [#13598] The DECLARED protocol contracts this domain's request literals are @@ -269,7 +270,7 @@ function requireManageMetadata(deps: DomainHandlerDeps, context: HttpProtocolCon * write cohort: an `organization_admin` holding `setup.access` (but not * `manage_metadata`) may inspect a package yet not publish or delete it, and a * write-only caller holding `manage_metadata` alone is refused these reads — - * pinned in `packages/rest/src/package-envelope.conformance.test.ts`. (#7020 + * pinned in `packages-capability-gate.test.ts`, beside this file. (#7020 * unified the object-schema MASK exemption with the write gate; it did not * re-rule this cohort, which is why this site names the read-only half.) * @@ -392,9 +393,8 @@ function requireWritablePackage( * `copied: []` that means "this gesture does not apply here" was byte-identical * to one meaning "the base really is empty". * - * That is the #11063 ruling, one route over and pointed at a WRITE: - * `packages/rest/src/package-routes.ts` states it verbatim — **"a read that - * could not happen must not be reported as a read that found nothing."** + * That is the #11063 ruling, one route over and pointed at a WRITE — **"a read + * that could not happen must not be reported as a read that found nothing."** * * ## Why the refusal, rather than teaching duplicate to clone code items * @@ -1271,8 +1271,9 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // given a shape nothing checks") — so `resolveService` hands this door an // `any`. That `any` is what let the call below send keys no declared shape // named: `organizationId` (the key that decides an uninstall's blast radius) - // and `keepData` are exactly the two the sibling REST door's option type - // could not express, and nothing compared the two doors' requests. Narrowed + // and `keepData` are exactly the two the since-removed REST twin's option + // type (#14503) could not express, and nothing compared the two doors' + // requests. Narrowed // to the producer's declared verb, so what this door sends is checked // against the contract the implementation states. // @@ -1315,12 +1316,14 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // digging into `persisted`) recorded an uninstall that had not // happened. // - // The failure rule is the one the REST twin of this route already - // uses (`packages/rest/src/package-routes.ts`), stated the same way - // on purpose — DELETE /packages/:id has TWO doors (this dispatcher - // and the direct-mount REST registrar, which shadows it only when a - // `package` service is registered), and two doors answering one - // request differently is how this divergence arrived. Zero metadata + // The failure rule is the one the REST twin of this route stated + // (`packages/rest/src/package-routes.ts`, until #14503 removed that + // twin), kept in the same words on purpose — DELETE /packages/:id + // had TWO doors (this dispatcher and the direct-mount REST + // registrar), and two doors answering one request differently is + // how this divergence arrived. Since #14503 this domain is the + // single implementation of the route; the rule stays because it is + // right, not because a twin still needs matching. Zero metadata // rows is still a successful uninstall — a runtime-registered // package that never published metadata has nothing in // `sys_metadata` — so only PER-ITEM failures make it a failure. diff --git a/scripts/check-undeclared-dep-imports.mjs b/scripts/check-undeclared-dep-imports.mjs index ec42db46be..04e13d7dbe 100644 --- a/scripts/check-undeclared-dep-imports.mjs +++ b/scripts/check-undeclared-dep-imports.mjs @@ -320,22 +320,6 @@ const LEDGER = [ + 'the same seam for exactly that reason — so declaring it would reverse a stance the tree ' + 'states, not repair an omission.', }, - { - pkg: '@objectstack/rest', - dep: '@objectstack/metadata-protocol', - file: 'packages/rest/src/package-routes.ts', - kind: 'type-only', - why: - '#9960 chose the PRODUCER\'s exported types over a local restatement for the ' - + '`protocol.deletePackage` seam, and refused a spec shape for it (zero external consumers). ' - + 'Nothing reaches the emitted JavaScript and rollup-plugin-dts inlines the two aliases, so an ' - + 'installing consumer is never told to install a package it does not receive. Measured on ' - + 'this branch: `packages/rest/dist/index.d.ts` and `index.d.cts` carry ZERO module ' - + 'references to `@objectstack/metadata-protocol` (its one textual occurrence is inside a ' - + 'TSDoc comment), and every `from` specifier in those published types names a package rest ' - + 'DECLARES — @objectstack/core, @objectstack/spec/* and zod. The row\'s mechanical evidence ' - + 'is the type-only form, checked every run; a value import ends it.', - }, ]; const LEDGER_KINDS = new Set(['optional-runtime-probe', 'type-only']); diff --git a/scripts/doc-authoring-prose-id.baseline.json b/scripts/doc-authoring-prose-id.baseline.json index 1ad330ad58..d92ce4e629 100644 --- a/scripts/doc-authoring-prose-id.baseline.json +++ b/scripts/doc-authoring-prose-id.baseline.json @@ -827,7 +827,6 @@ "#11924": 2, "#12038": 9, "#12702": 4, - "#2747": 1, "#3610": 1, "#3611": 1, "#4327": 1,