diff --git a/.changeset/scoped-packages-dispatcher-door.md b/.changeset/scoped-packages-dispatcher-door.md new file mode 100644 index 0000000000..2036027c13 --- /dev/null +++ b/.changeset/scoped-packages-dispatcher-door.md @@ -0,0 +1,18 @@ +--- +"@objectstack/runtime": minor +--- + +fix(runtime): mount the scoped `/api/v1/environments/:id/packages*` door, and reconcile the package read/delete responses to their declared schemas (#16781) + +**The door.** `mountPackagesRoute` mounted `/packages*` at the unscoped prefix only, while automation / actions / ai each registered a scoped variant twenty lines away. On a host composed as `@objectstack/plugin-hono-server` + this plugin with `enableProjectScoping: true` and **without** `@objectstack/hono`'s `createHonoApp`, that left `GET /api/v1/environments/:id/packages`, `GET …/packages/:id` and `DELETE …/packages/:id` answered by the transport's own `notFound` — a bare 404 on routes `content/docs/api/environment-routing.mdx` documents. The domain has resolved scoped package paths since #15859; nothing mounted one. + +`mountPackagesRoute` is now wrapped in a `base`-taking `registerPackageRoutes(base)`, exactly like its three siblings, and called a second time with the scoped base. **The same handler, no second implementation.** The unscoped mounts keep their registration position and their unconditional mounting, so the change is purely additive: no route that answered before stops answering. + +**The wire.** Two responses gained the key their own declared schema requires (contract review of #16628, finding F2). Both additions are **additive** — no key left either payload: + +- `GET /packages` now sends **`hasMore`** (`ListInstalledPackagesResponseSchema`). It is `false`: this door applies its `status` / `type` filters and returns every remaining row, reading no `limit` and no `cursor`, so there is no next page to announce. +- `DELETE /packages/:id` now sends **`packageId`** (`UninstallPackageApiResponseSchema`). `registryRemoved` and `persisted` stay on the wire unchanged. + +A client that reads only the keys it read before is unaffected; a client parsing either payload against the published schema stops being refused. + +The `DELETE /packages/:id` route-ledger row now carries `responseSchema: 'UninstallPackageApiResponseSchema'`, backed by new conformance coverage that drives the real handler. `GET /packages` is deliberately left blank: its rows are the ASSEMBLED package body, while `InstalledPackageSchema` wraps the AUTHORING-stage `ManifestSchema` — the #14242 stage mismatch, which no `@objectstack/spec/api` export declares yet. Both directions of that boundary are pinned, so the row becomes fillable against a red test rather than a guess. diff --git a/packages/runtime/src/dispatcher-plugin.scoped-packages-door.integration.test.ts b/packages/runtime/src/dispatcher-plugin.scoped-packages-door.integration.test.ts new file mode 100644 index 0000000000..1d07b11862 --- /dev/null +++ b/packages/runtime/src/dispatcher-plugin.scoped-packages-door.integration.test.ts @@ -0,0 +1,155 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16781 — the scoped `/packages` door on a `plugin-hono-server`-only host. + * + * ## The composition this file exists for + * + * A host composed as `plugin-hono-server` + the dispatcher with + * `enableProjectScoping: true`, and **without** `@objectstack/hono`'s + * `createHonoApp`, has exactly two ways a request can reach the `/packages` + * domain: an explicit route this plugin mounts, or `createHonoApp`'s + * `app.all(`${prefix}/*`)` catch-all — which this composition does not have. + * `setFallbackHandler` is not a third: it is gated on `isAppEndpointPath`, so + * a scoped package URL never reaches it. + * + * Before #16781 the plugin mounted `/packages*` at the UNSCOPED prefix only — + * automation / actions / ai each had a scoped variant twenty lines away and + * packages had none — so `GET /api/v1/environments/:id/packages` on this + * composition was answered by the transport's own `notFound`. The domain + * itself has handled scoped paths since #15859 + * (`packages-single-door.test.ts` pins that half); what was missing was the + * MOUNT, and only a test that boots this composition over a real socket can + * see the difference. + * + * ## The acceptance control, verbatim from the card + * + * > the same request on the same composition answers `ROUTE_NOT_FOUND`/bare + * > 404 before and the dispatcher's row after. + * + * `dispatcherAnswered()` below is the discriminator, and it is a positive + * test rather than "not a 404": the anonymous-deny floor (#7033/#7023) is the + * FIRST statement in `handlePackagesRequest`, ahead of the registry probe, so + * a credential-less request that REACHES the dispatcher is answered + * `ANONYMOUS_DENY_STATUS` / `ANONYMOUS_DENY_CODE` — a verdict no + * transport-level sink emits, since an unmounted path never gets past + * `notFound`. The two constants are IMPORTED rather than spelled, so a rename + * moves this file with them instead of quietly turning the discriminator into + * a literal that nothing produces. The negative control immediately below + * drives an unmounted sibling path through the same assertion and shows it + * answering the transport's own 404 instead, so the discriminator is measured + * in both directions rather than assumed. + * + * ## Why no credentials + * + * The claim under test is "a door exists here", and the anonymous floor is the + * earliest observable proof of arrival — earlier than the 503 an unprovisioned + * registry would give, and it cannot be produced by the transport. Provisioning + * an authenticated caller would move the assertion downstream of two more gates + * without making it say more about the mount. The RESPONSE SHAPES this card + * also reconciles are pinned where they can be parsed against the spec, in + * `domains/packages-read-delete-response-conformance.test.ts`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_STATUS, LiteKernel } from '@objectstack/core'; +import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; +import type { IHttpServer } from '@objectstack/spec/contracts'; + +import { createDispatcherPlugin } from './dispatcher-plugin.js'; + +const PREFIX = '/api/v1'; +const ENV_ID = 'env_alpha'; +const PKG_ID = 'com.acme.crm'; + +let kernel: LiteKernel | undefined; +let baseUrl = ''; + +/** + * The composition named on the card: the hono TRANSPORT plugin plus the + * dispatcher, scoping on. No `createHonoApp`, no `@objectstack/rest`, and no + * service plugins — nothing here may supply a second door. + */ +beforeAll(async () => { + kernel = new LiteKernel(); + kernel.use(new HonoServerPlugin({ port: 0, cors: false })); + kernel.use(createDispatcherPlugin({ + prefix: PREFIX, + scoping: { enableProjectScoping: true, projectResolution: 'auto' }, + enforceProjectMembership: false, + securityHeaders: false, + })); + await kernel.bootstrap(); + const httpServer = kernel.getService('http.server'); + baseUrl = `http://127.0.0.1:${httpServer.getPort!()}`; +}, 60_000); + +afterAll(async () => { + if (!kernel) return; + await Promise.race([ + kernel.shutdown(), + new Promise((resolve) => setTimeout(resolve, 10_000)), + ]); +}, 60_000); + +async function probe(method: string, path: string): Promise<{ status: number; body: any }> { + const res = await fetch(`${baseUrl}${path}`, { method }); + let body: any; + try { body = await res.json(); } catch { body = undefined; } + return { status: res.status, body }; +} + +/** + * Did the DISPATCHER answer this request? + * + * The anonymous deny is minted inside `dispatcher.dispatch()` and by nothing + * in the transport, so a true reading here means the request crossed the + * mount. This is the card's "the dispatcher's row", stated as the thing that + * is observable without credentials. + */ +function dispatcherAnswered(r: { status: number; body: any }): boolean { + return r.status === ANONYMOUS_DENY_STATUS && r.body?.error?.code === ANONYMOUS_DENY_CODE; +} + +/** The shape "no door answered" takes on this transport. */ +function transportRefused(r: { status: number; body: any }): boolean { + const code = r.body?.error?.code; + return r.status === 404 && (code === undefined || code === 'ROUTE_NOT_FOUND' || code === 'ENDPOINT_NOT_FOUND'); +} + +const SCOPED = `${PREFIX}/environments/${ENV_ID}/packages`; +const UNSCOPED = `${PREFIX}/packages`; + +/** The three routes the card names, plus the verb each is reached by. */ +const CARD_ROUTES: Array<[string, string]> = [ + ['GET', ''], + ['GET', `/${PKG_ID}`], + ['DELETE', `/${PKG_ID}`], +]; + +describe('#16781 — the discriminator itself, measured in both directions', () => { + it('POSITIVE CONTROL: the UNSCOPED door has always existed and answers from the domain', async () => { + for (const [method, sub] of CARD_ROUTES) { + const r = await probe(method, `${UNSCOPED}${sub}`); + expect(dispatcherAnswered(r), `${method} ${UNSCOPED}${sub} -> ${r.status} ${JSON.stringify(r.body)}`).toBe(true); + } + }, 60_000); + + it('NEGATIVE CONTROL: a scoped path no mount claims answers the transport, not the domain', async () => { + const r = await probe('GET', `${PREFIX}/environments/${ENV_ID}/no-such-domain`); + expect(dispatcherAnswered(r)).toBe(false); + expect(transportRefused(r), `unmounted sibling -> ${r.status} ${JSON.stringify(r.body)}`).toBe(true); + }, 60_000); +}); + +describe('#16781 — the scoped /packages door on a plugin-hono-server-only composition', () => { + for (const [method, sub] of CARD_ROUTES) { + it(`${method} ${SCOPED}${sub} answers through the dispatcher`, async () => { + const r = await probe(method, `${SCOPED}${sub}`); + expect( + dispatcherAnswered(r), + `${method} ${SCOPED}${sub} -> ${r.status} ${JSON.stringify(r.body)}`, + ).toBe(true); + }, 60_000); + } +}); diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 4ae5006242..b78584d412 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -1298,52 +1298,78 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu // directly, which skipped that pipeline entirely and dropped // req.query on several routes (so the documented `?overwrite=true` // install flag never reached the handler). - const mountPackagesRoute = ( - verb: 'get' | 'post' | 'patch' | 'delete', - routePath: string, - toSubPath: (req: any) => string, - ) => { - (server as any)[verb](`${prefix}/packages${routePath}`, async (req: any, res: any) => { - try { - const result = await dispatcher.dispatch( - verb.toUpperCase(), - `/packages${toSubPath(req)}`, - req.body, - req.query ?? {}, - { request: req }, - ); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); + // + // [#16781] A `base`-taking registrar, exactly like + // `registerAutomationRoutes` / `registerActionRoutes` / + // `registerAIRoutes` below, so the SAME handler can be mounted at + // the environment-scoped prefix as well. It used to close over + // `prefix` directly, and that is the whole reason + // `/api/v1/environments/:id/packages` had no door on a host + // composed as `plugin-hono-server` + this plugin WITHOUT + // `@objectstack/hono`'s catch-all: the domain has resolved scoped + // package paths since #15859, but nothing mounted one here. The + // scoped registration is at the `enableProjectScoping` block below, + // beside its three siblings; `dispatch()` is still handed the + // UNSCOPED subpath, and the `:environmentId` rides on `req.params` + // for `prepareResolverHints` to read — the same convention the + // action routes document. + const registerPackageRoutes = (base: string) => { + const mountPackagesRoute = ( + verb: 'get' | 'post' | 'patch' | 'delete', + routePath: string, + toSubPath: (req: any) => string, + ) => { + (server as any)[verb](`${base}/packages${routePath}`, async (req: any, res: any) => { + try { + const result = await dispatcher.dispatch( + verb.toUpperCase(), + `/packages${toSubPath(req)}`, + req.body, + req.query ?? {}, + { request: req }, + ); + sendResult(result, res); + } catch (err: any) { + errorResponse(err, res); + } + }); + }; + + mountPackagesRoute('get', '', () => ''); + mountPackagesRoute('post', '', () => ''); + mountPackagesRoute('get', '/:id/export', (req) => `/${req.params.id}/export`); + mountPackagesRoute('get', '/:id', (req) => `/${req.params.id}`); + mountPackagesRoute('delete', '/:id', (req) => `/${req.params.id}`); + // Edit a package's manifest (name / description / version). `/:id` + // is a single segment, so this does not shadow the + // `/:id/enable|disable` routes below. + mountPackagesRoute('patch', '/:id', (req) => `/${req.params.id}`); + mountPackagesRoute('patch', '/:id/enable', (req) => `/${req.params.id}/enable`); + mountPackagesRoute('patch', '/:id/disable', (req) => `/${req.params.id}/disable`); + mountPackagesRoute('post', '/:id/publish', (req) => `/${req.params.id}/publish`); + // ADR-0033 — publish every pending draft bound to a package ("publish + // whole app"). Distinct from /publish (which needs the metadata + // service): this promotes sys_metadata draft rows via the protocol. + mountPackagesRoute('post', '/:id/publish-drafts', (req) => `/${req.params.id}/publish-drafts`); + mountPackagesRoute('post', '/:id/revert', (req) => `/${req.params.id}/revert`); + // duplicate (ADR-0070 D4), adopt-orphans (D5), discard-drafts, and + // the ADR-0067 commit-history / rollback family. + mountPackagesRoute('post', '/:id/duplicate', (req) => `/${req.params.id}/duplicate`); + mountPackagesRoute('post', '/:id/adopt-orphans', (req) => `/${req.params.id}/adopt-orphans`); + mountPackagesRoute('post', '/:id/discard-drafts', (req) => `/${req.params.id}/discard-drafts`); + mountPackagesRoute('get', '/:id/commits', (req) => `/${req.params.id}/commits`); + mountPackagesRoute('post', '/:id/commits/:commitId/revert', (req) => `/${req.params.id}/commits/${req.params.commitId}/revert`); + mountPackagesRoute('post', '/:id/rollback', (req) => `/${req.params.id}/rollback`); }; - mountPackagesRoute('get', '', () => ''); - mountPackagesRoute('post', '', () => ''); - mountPackagesRoute('get', '/:id/export', (req) => `/${req.params.id}/export`); - mountPackagesRoute('get', '/:id', (req) => `/${req.params.id}`); - mountPackagesRoute('delete', '/:id', (req) => `/${req.params.id}`); - // Edit a package's manifest (name / description / version). `/:id` - // is a single segment, so this does not shadow the - // `/:id/enable|disable` routes below. - mountPackagesRoute('patch', '/:id', (req) => `/${req.params.id}`); - mountPackagesRoute('patch', '/:id/enable', (req) => `/${req.params.id}/enable`); - mountPackagesRoute('patch', '/:id/disable', (req) => `/${req.params.id}/disable`); - mountPackagesRoute('post', '/:id/publish', (req) => `/${req.params.id}/publish`); - // ADR-0033 — publish every pending draft bound to a package ("publish - // whole app"). Distinct from /publish (which needs the metadata - // service): this promotes sys_metadata draft rows via the protocol. - mountPackagesRoute('post', '/:id/publish-drafts', (req) => `/${req.params.id}/publish-drafts`); - mountPackagesRoute('post', '/:id/revert', (req) => `/${req.params.id}/revert`); - // duplicate (ADR-0070 D4), adopt-orphans (D5), discard-drafts, and - // the ADR-0067 commit-history / rollback family. - mountPackagesRoute('post', '/:id/duplicate', (req) => `/${req.params.id}/duplicate`); - mountPackagesRoute('post', '/:id/adopt-orphans', (req) => `/${req.params.id}/adopt-orphans`); - mountPackagesRoute('post', '/:id/discard-drafts', (req) => `/${req.params.id}/discard-drafts`); - mountPackagesRoute('get', '/:id/commits', (req) => `/${req.params.id}/commits`); - mountPackagesRoute('post', '/:id/commits/:commitId/revert', (req) => `/${req.params.id}/commits/${req.params.commitId}/revert`); - mountPackagesRoute('post', '/:id/rollback', (req) => `/${req.params.id}/rollback`); + // Mounted at the UNSCOPED prefix right here, keeping the exact + // registration ORDER these routes have always had — Hono resolves + // competing patterns first-registration-wins (the ADR-0076 D11 + // hazard this file's fallback note explains), so moving this call + // down beside the scoped one would be a behaviour change wearing a + // refactor's clothes. The scoped mount is purely ADDITIVE and + // cannot shadow anything: it lives under a different path prefix. + registerPackageRoutes(prefix); // ── Storage ───────────────────────────────────────────────── // Nothing mounted here on purpose (#4087). The dispatcher used to @@ -1701,6 +1727,23 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu } } + // [#16781] The scoped `/packages` door, the residue PR #16628 was + // authorised to leave behind (ruling C′ on #14503 step 2). Same + // handler as the unscoped mount above — `registerPackageRoutes` is + // called a second time with the scoped base, never re-implemented. + // + // ONE condition rather than the three-way branch its siblings take, + // and the difference is deliberate: `registerAutomationRoutes` / + // `registerActionRoutes` / `registerAIRoutes` DROP their unscoped + // mounts under `projectResolution: 'required'`, while the package + // routes above are mounted unconditionally and stay that way. This + // card adds a missing door; taking one away is a different change + // with a different blast radius, so the asymmetry is left standing + // and recorded here rather than silently "tidied" into a removal. + if (enableProjectScoping) { + registerPackageRoutes(`${prefix}/environments/:environmentId`); + } + ctx.logger.info('Dispatcher bridge routes registered', { prefix, enableProjectScoping, projectResolution }); // ── Declarative endpoint mount seam (#5040 E3) ─────────────── diff --git a/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts b/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts new file mode 100644 index 0000000000..22e7fa1d2f --- /dev/null +++ b/packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts @@ -0,0 +1,302 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16781 deliverable 2 — the payloads `GET /packages` and + * `DELETE /packages/:id` actually serve, parsed against the contracts + * `@objectstack/spec/api` declares for them. + * + * ## What was measured, and why this file exists + * + * The contract review of PR #16628 (comment 5578894182, finding F2) measured + * two doors answering shapes their own declared schemas refuse: + * + * - `GET /packages` answered `{ packages, total }`, while + * `ListInstalledPackagesResponseSchema` requires `hasMore`; + * - `DELETE /packages/:id` answered `{ success, registryRemoved, persisted }`, + * while `UninstallPackageApiResponseSchema` requires `packageId`. + * + * Both were reconciled toward the SPEC (protocol is the baseline) and both + * reconciliations are ADDITIVE: a key each door was missing was added, and no + * key that was on either wire left it. This file is the conformance coverage + * `route-ledger.ts`'s `responseSchema` field is forbidden to be written + * without — 「⛔ DO NOT FILL A ROW THAT HAS NO CONFORMANCE COVERAGE」 — and it + * is also what says WHICH of the two rows may carry a name and which may not. + * + * ## The declared surface here is the WHOLE BODY, not the `data` + * + * Unlike the `/packages` lifecycle rows (`PublishPackageDraftsResponseSchema` + * and friends), which declare the `data` payload alone, both schemas here are + * `BaseResponseSchema.extend({ data })` — they name the envelope AND its + * payload. So the parses below are handed `body`, not `body.data`, and a + * regression in the envelope reddens here too. + * + * ## ⚠️ The list row's remaining gap is the #14242 STAGE mismatch, not F2 + * + * F2 was not the only thing standing between `GET /packages` and its declared + * schema, and this file measures the rest rather than declaring past it. + * `ListInstalledPackagesResponseSchema` types each row as + * `InstalledPackageSchema`, whose `manifest` is `ManifestSchema` — the + * AUTHORING-stage manifest, where `objects` is an array of GLOB PATTERNS. What + * the registry stores, and therefore what this door serves, is the ASSEMBLED + * body: `ObjectQL.registerApp` is handed `manifest.objects` as object + * DEFINITIONS and `SchemaRegistry.installPackage` records what it was given. + * + * That is the mismatch #14242 identified one layer down, whose maintainer + * ruling (2026-09-02, quoted in `stack.zod.ts` at `ArtifactPackageSchema`) was + * to «declare the assembled stage rather than widen the authoring one». No + * assembled-stage counterpart of `InstalledPackageSchema` exists in + * `@objectstack/spec/api` yet, and authoring one is a `packages/spec` change + * this card is explicitly routed away from. + * + * ⇒ The `GET /packages` ledger row is deliberately left WITHOUT a + * `responseSchema`. Writing one would be exactly the "declared but unverified" + * surface the ledger header exists to prevent: it would read as a promise the + * door keeps only for glob-authored packages and breaks for every + * `defineStack()` host, which is the shipped open-core path. The boundary is + * pinned below in BOTH directions, so whoever declares the assembled stage + * gets a red test telling them the row has become fillable. + * + * ## The residue on the delete row, PINNED rather than hidden + * + * `UninstallPackageApiResponseSchema` does not carry `registryRemoved` or + * `persisted`, which the delete door really serves, so a declared parse + * STRIPS them. Deleting live keys from a published payload to make the parse + * total is a wire removal and out of this card's scope; widening the schema is + * again `packages/spec`. So the gap is asserted BY NAME: a measured, bounded + * residue, where either side moving turns this red instead of drifting. + * + * ## Harness + * + * Borrowed from `packages-single-door.test.ts`: a REAL `SchemaRegistry` with a + * real package installed, and identity through the real resolver (the kernel + * offers an `auth` session and an ObjectQL engine whose `find` answers the + * permission-set tables), so the rows under test are the rows the door builds + * and the gates inside the domain run rather than being bypassed. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { SchemaRegistry } from '@objectstack/objectql'; +import { + ListInstalledPackagesResponseSchema, + UninstallPackageApiResponseSchema, +} from '@objectstack/spec/api'; +import { HttpDispatcher, type HttpDispatcherResult } from '../http-dispatcher.js'; + +const PREFIX = '/api/v1'; + +/** + * The GLOB-authored manifest — the AUTHORING stage `ManifestSchema` declares, + * where `objects` names file patterns. + */ +const GLOB_PKG = { + id: 'com.acme.glob', namespace: 'glob', version: '1.0.0', type: 'app', scope: 'project', + name: 'Glob Authored', objects: ['./src/objects/*.object.yml'], +}; + +/** + * The ASSEMBLED manifest a `defineStack()` host produces — `objects` carries + * object DEFINITIONS, which is the payload `ObjectQL.registerApp` iterates. + * This is the shipped open-core shape. + */ +const CODE_PKG = { + id: 'com.acme.code', namespace: 'code', version: '1.0.0', type: 'app', scope: 'project', + name: 'Code Defined', objects: [{ name: 'code_lead', fields: { title: { type: 'text' } } }], +}; + +/** 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'] }, + ], +}; + +/** + * 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; +} + +/** + * @param manifests packages to install into the real registry. + * @param protocol optional `protocol` slot; supplied for the delete cases so + * `persisted` is a REAL object on the wire rather than the + * `undefined` a JSON round-trip would silently drop. + */ +function dispatcher(manifests: any[], protocol?: unknown): HttpDispatcher { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' }); + (registry as any).logLevel = 'silent'; + for (const m of manifests) registry.installPackage(m as 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, ...(protocol ? { protocol } : {}) }; + return new HttpDispatcher({ + getState: () => 'running', + getService: (n: string) => services[n], + getServiceAsync: async (n: string) => services[n], + } as any); +} + +function responseOf(res: HttpDispatcherResult, what: string): NonNullable { + const { response } = res; + if (!response) throw new Error(`${what} answered no response at all`); + return response; +} + +/** + * Drive the door and read back what a CLIENT would see. + * + * The JSON round-trip is load-bearing rather than ceremonial: an in-process + * object carries an `undefined` member as a present key while the wire does + * not, and the conformance claim is about the wire. + */ +async function send( + method: string, url: string, manifests: any[], protocol?: unknown, +): Promise<{ status: number; body: any }> { + const ctx: any = { request: new Request(`http://pin.local${url}`, { method }) }; + const res = await dispatcher(manifests, protocol) + .dispatch(method, url.substring(PREFIX.length), undefined, {}, ctx, PREFIX); + const response = responseOf(res, `${method} ${url}`); + return { status: response.status, body: JSON.parse(JSON.stringify(response.body)) }; +} + +/** Keys of `raw` that the declared parse refused to carry through. */ +function strippedKeys(raw: Record, parsed: Record): string[] { + return Object.keys(raw).filter((k) => !(k in parsed)); +} + +describe('#16781 — GET /packages: the F2 gap is closed on EVERY authoring path', () => { + for (const [label, pkg] of [['glob-authored', GLOB_PKG], ['assembled / defineStack', CODE_PKG]] as const) { + it(`${label}: \`hasMore\` — the key F2 measured missing — is on the wire`, async () => { + const r = await send('GET', `${PREFIX}/packages`, [pkg]); + expect(r.status).toBe(200); + expect(r.body.data.hasMore).toBe(false); + expect(r.body.data.total).toBe(1); + }); + + it(`${label}: the \`data\` envelope carries ONLY keys the contract declares`, async () => { + const r = await send('GET', `${PREFIX}/packages`, [pkg]); + // Read off the declaration rather than restated: `nextCursor` is + // optional and absent (this door does not paginate), so the served + // set is exactly these three. + expect(Object.keys(r.body.data).sort()).toEqual(['hasMore', 'packages', 'total']); + }); + } + + it('a glob-authored row parses END TO END — this is the shape the contract describes', async () => { + const r = await send('GET', `${PREFIX}/packages`, [GLOB_PKG]); + const parsed = ListInstalledPackagesResponseSchema.parse(r.body); + expect(parsed.success).toBe(true); + expect(parsed.data.hasMore).toBe(false); + expect(parsed.data.packages).toHaveLength(1); + expect(strippedKeys(r.body.data, parsed.data as any)).toEqual([]); + }); + + it('`hasMore` is what closed F2: the pre-#16781 body is REFUSED', async () => { + const r = await send('GET', `${PREFIX}/packages`, [GLOB_PKG]); + // The exact payload this door served before the reconciliation, built + // by deleting the one key that was added — so this is a statement + // about the fix, not about a hand-written literal. + const before = { ...r.body, data: { ...r.body.data } }; + delete before.data.hasMore; + + const verdict = ListInstalledPackagesResponseSchema.safeParse(before); + expect(verdict.success).toBe(false); + expect(verdict.error!.issues.some((i) => i.path.join('.') === 'data.hasMore')).toBe(true); + }); + + /** + * ⭐ THE BOUNDARY, and the reason the `GET /packages` ledger row carries no + * `responseSchema`. + * + * This asserts a CURRENT FAILURE on purpose. On the shipped `defineStack()` + * path the served row still does not parse — and the surviving issue is + * `manifest.objects` ALONE, the #14242 authoring-vs-assembled stage + * mismatch, with `data.hasMore` gone from the issue list because this card + * closed it. When someone declares the assembled stage (the ruled remedy), + * this test goes red and tells them the row has become fillable. + */ + it('the assembled row does NOT yet parse, and the ONLY surviving issue is the #14242 stage mismatch', async () => { + const r = await send('GET', `${PREFIX}/packages`, [CODE_PKG]); + const verdict = ListInstalledPackagesResponseSchema.safeParse(r.body); + + expect(verdict.success).toBe(false); + expect(verdict.error!.issues.map((i) => i.path.join('.'))) + .toEqual(['data.packages.0.manifest.objects.0']); + }); +}); + +describe('#16781 — DELETE /packages/:id conforms to UninstallPackageApiResponseSchema', () => { + /** A `protocol` slot whose `deletePackage` answers the clean-uninstall shape. */ + const protocolStub = () => ({ + deletePackage: vi.fn().mockResolvedValue({ + success: true, deletedCount: 2, failedCount: 0, failed: [], cleanups: [], + }), + }); + + const del = (pkg: any = CODE_PKG) => + send('DELETE', `${PREFIX}/packages/${pkg.id}`, [pkg], protocolStub()); + + it('the served body parses, and `packageId` — the key F2 measured missing — names the package', async () => { + const r = await del(); + expect(r.status).toBe(200); + + const parsed = UninstallPackageApiResponseSchema.parse(r.body); + expect(parsed.success).toBe(true); + expect(parsed.data.packageId).toBe(CODE_PKG.id); + expect(parsed.data.success).toBe(true); + }); + + it('it parses on BOTH authoring paths — this row carries no manifest, so #14242 cannot reach it', async () => { + const parsed = UninstallPackageApiResponseSchema.parse((await del(GLOB_PKG)).body); + expect(parsed.data.packageId).toBe(GLOB_PKG.id); + }); + + it('`packageId` is what closed F2: the pre-#16781 body is REFUSED', async () => { + const r = await del(); + const before = { ...r.body, data: { ...r.body.data } }; + delete before.data.packageId; + + const verdict = UninstallPackageApiResponseSchema.safeParse(before); + expect(verdict.success).toBe(false); + expect(verdict.error!.issues.some((i) => i.path.join('.') === 'data.packageId')).toBe(true); + }); + + it('the UNDECLARED residue is exactly `registryRemoved` + `persisted` — named, not hidden', async () => { + const r = await del(); + const parsed: any = UninstallPackageApiResponseSchema.parse(r.body); + + // The door serves these; the schema does not carry them, so a declared + // parse drops them. Removing them from the wire is a payload DELETION + // and out of this card's scope; widening the schema is a + // `packages/spec` change this card is routed away from. Asserted by + // name so the gap stays a measured fact rather than a silent one. + expect(strippedKeys(r.body.data, parsed.data)).toEqual(['registryRemoved', 'persisted']); + }); +}); diff --git a/packages/runtime/src/domains/packages-single-door.test.ts b/packages/runtime/src/domains/packages-single-door.test.ts index 5a791550c3..c1df0603a9 100644 --- a/packages/runtime/src/domains/packages-single-door.test.ts +++ b/packages/runtime/src/domains/packages-single-door.test.ts @@ -29,7 +29,9 @@ * - 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 list answers `{ packages, total, hasMore }` (`hasMore` added by + * #16781, reconciling the door to `ListInstalledPackagesResponseSchema`) + * 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 @@ -201,11 +203,13 @@ describe('/packages — one implementation, and its 404 wording says which (#145 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 () => { + it(`${label} GET /packages answers { packages, total, hasMore } 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); + // [#16781] Part of "which door answered": the declared key set. + expect(r.body?.data?.hasMore).toBe(false); 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); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index cf8ed623be..5716ac29c3 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -596,7 +596,20 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin const rows = packages.map( (p: any) => withWritableVerdict(qlService, toPackageResponse(p) as any), ); - return { handled: true, response: deps.success({ packages: rows, total: rows.length }) }; + // [#16781] `hasMore` is REQUIRED by + // `ListInstalledPackagesResponseSchema` and this door did not send + // it, so the payload it served could not parse through its own + // declared contract. Reconciled toward the SPEC (protocol is the + // baseline), additively — nothing that was on this wire left it. + // + // The value is a constant `false` because it is TRUE, not because + // it is convenient: this door applies the `status` / `type` + // filters and then returns every remaining row. It reads no + // `limit` and no `cursor`, so there is never a next page to + // announce and `nextCursor` (optional) stays absent. If this route + // ever starts paginating, `hasMore` is the key that has to start + // telling the truth — which is exactly why it is declared. + return { handled: true, response: deps.success({ packages: rows, total: rows.length, hasMore: false }) }; } // POST /packages → install package. @@ -1351,7 +1364,16 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin if (!registryRemoved && deletedCount === 0) { return { handled: true, response: deps.error(`Package '${id}' not found`, 404) }; } - return { handled: true, response: deps.success({ success: true, registryRemoved, persisted }) }; + // [#16781] `packageId` is REQUIRED by + // `UninstallPackageApiResponseSchema` and this door did not send + // it. Added, additively — `registryRemoved` and `persisted` stay + // on the wire exactly as they were. They are keys the declared + // schema does not carry, so a declared parse STRIPS them; that + // residue is pinned by name in + // `packages-read-delete-response-conformance.test.ts` rather than + // fixed by deleting live keys from a published payload, which is + // not this card's to do. + return { handled: true, response: deps.success({ packageId: id, success: true, registryRemoved, persisted }) }; } } catch (e: any) { return { handled: true, response: deps.errorFromThrown(e, 500) }; diff --git a/packages/runtime/src/route-ledger.ts b/packages/runtime/src/route-ledger.ts index 3a66f84341..289494e22d 100644 --- a/packages/runtime/src/route-ledger.ts +++ b/packages/runtime/src/route-ledger.ts @@ -374,10 +374,27 @@ export const ROUTE_LEDGER: readonly RouteLedgerEntry[] = [ { route: 'GET /share-links/:token/messages', domain: '/share-links', disposition: 'public', note: 'unauthenticated shared-conversation messages' }, // ── packages ────────────────────────────────────────────────────────────── + // [#16781] NO `responseSchema`, and the blank is a MEASURED verdict rather + // than an unvisited row. `ListInstalledPackagesResponseSchema` now describes + // the envelope this door serves — the missing `hasMore` (#16628 contract + // review F2) was added — but it types each row as `InstalledPackageSchema`, + // whose `manifest` is the AUTHORING-stage `ManifestSchema` (`objects` = glob + // patterns). This door serves the ASSEMBLED body, where `objects` carries + // object DEFINITIONS: the #14242 stage mismatch, whose maintainer ruling + // (2026-09-02, quoted at `ArtifactPackageSchema` in spec `stack.zod.ts`) was + // to declare the assembled stage rather than widen the authoring one. Until + // an assembled-stage counterpart exists in `@objectstack/spec/api`, a name + // here would promise conformance the door keeps only for glob-authored + // packages and breaks for every `defineStack()` host — the "declared but + // unverified" surface this field's header forbids. Both directions of that + // boundary are pinned in `domains/packages-read-delete-response-conformance.test.ts`, + // so the row becomes fillable against a RED test, never against a guess. { route: 'GET /packages', domain: '/packages', disposition: 'sdk', client: 'packages.list' }, { route: 'POST /packages', domain: '/packages', disposition: 'sdk', client: 'packages.install' }, { route: 'GET /packages/:id', domain: '/packages', disposition: 'sdk', client: 'packages.get' }, - { route: 'DELETE /packages/:id', domain: '/packages', disposition: 'sdk', client: 'packages.uninstall' }, + { route: 'DELETE /packages/:id', domain: '/packages', disposition: 'sdk', client: 'packages.uninstall', + responseSchema: 'UninstallPackageApiResponseSchema', + note: 'The schema names the WHOLE BODY here, envelope included (`BaseResponseSchema.extend({ data })`), not the `data` alone its lifecycle siblings above declare. Fillable because `domains/packages-read-delete-response-conformance.test.ts` drives THIS handler and parses the payload it answers, on both authoring paths — the row carries no manifest, so the authoring-vs-assembled manifest stage mismatch that keeps `GET /packages` blank cannot reach it. ⚠️ The declaration is a strict SUBSET of the wire: the door also serves `registryRemoved` and `persisted`, which the schema does not carry and a declared parse therefore strips. That residue is asserted by name in the same file rather than fixed — deleting live keys from a published payload is a wire removal, and widening the schema is a `packages/spec` change. See the comment above the `GET /packages` row for the stage mismatch and its ruling' }, { route: 'PATCH /packages/:id/enable', domain: '/packages', disposition: 'sdk', client: 'packages.enable' }, { route: 'PATCH /packages/:id/disable', domain: '/packages', disposition: 'sdk', client: 'packages.disable' }, { route: 'PATCH /packages/:id', domain: '/packages', disposition: 'sdk', client: 'packages.update' },