From a4af4eceb019c27fc0f8bcc2c7240651d0dac614 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 13:40:21 +0000 Subject: [PATCH 1/2] fix(runtime): mount the scoped /packages dispatcher door and reconcile its read/delete responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 `plugin-hono-server` + the dispatcher 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`. The domain has resolved scoped package paths since #15859; nothing mounted one. Wrap `mountPackagesRoute` in a `base`-taking `registerPackageRoutes(base)`, the same shape its three siblings use, and call it a second time with the scoped base. Same handler, no second implementation. The unscoped mounts keep their registration position and stay unconditional, so the change is additive. Reconcile both doors to their declared schemas (contract review of #16628, F2), additively — no key leaves either payload: - `GET /packages` sends `hasMore` (false: this door reads no limit/cursor and returns every row that survives its filters). - `DELETE /packages/:id` sends `packageId`. Fill `responseSchema` on the `DELETE /packages/:id` ledger row, backed by new conformance coverage driving the real handler. `GET /packages` is left blank on purpose: its rows carry the ASSEMBLED package body while `InstalledPackageSchema` wraps the AUTHORING-stage `ManifestSchema` (the #14242 stage mismatch), so a name there would be false on every `defineStack()` host. Both directions of that boundary are pinned. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt --- .changeset/scoped-packages-dispatcher-door.md | 18 ++ ...n.scoped-packages-door.integration.test.ts | 155 +++++++++ packages/runtime/src/dispatcher-plugin.ts | 131 +++++--- ...s-read-delete-response-conformance.test.ts | 302 ++++++++++++++++++ .../src/domains/packages-single-door.test.ts | 8 +- packages/runtime/src/domains/packages.ts | 26 +- packages/runtime/src/route-ledger.ts | 19 +- 7 files changed, 610 insertions(+), 49 deletions(-) create mode 100644 .changeset/scoped-packages-dispatcher-door.md create mode 100644 packages/runtime/src/dispatcher-plugin.scoped-packages-door.integration.test.ts create mode 100644 packages/runtime/src/domains/packages-read-delete-response-conformance.test.ts 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..66538d6a0a 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: '[#16781] 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 #14242 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' }, { 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' }, From 7c0629da4f8439de893671c98306527752394cb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 14:06:34 +0000 Subject: [PATCH 2/2] fix(runtime): strip tracker ids from the route-ledger note string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:doc-authoring` refuses new internal issue-id references in sibling-package string prose: a runtime string reaches authors, operators and generated surfaces, none of whom can resolve `#NNNN`. The ids move to the adjacent `//` comment, where the reader who CAN resolve them reads the source. Maintainer ruling 2026-08-12, verbatim: 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt --- packages/runtime/src/route-ledger.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime/src/route-ledger.ts b/packages/runtime/src/route-ledger.ts index 66538d6a0a..289494e22d 100644 --- a/packages/runtime/src/route-ledger.ts +++ b/packages/runtime/src/route-ledger.ts @@ -394,7 +394,7 @@ export const ROUTE_LEDGER: readonly RouteLedgerEntry[] = [ { route: 'GET /packages/:id', domain: '/packages', disposition: 'sdk', client: 'packages.get' }, { route: 'DELETE /packages/:id', domain: '/packages', disposition: 'sdk', client: 'packages.uninstall', responseSchema: 'UninstallPackageApiResponseSchema', - note: '[#16781] 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 #14242 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' }, + 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' },