From ece1b48a68f136567c4b0f48ff7f128004c1a612 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 13:06:50 +0000 Subject: [PATCH] feat(runtime): single owner for /discovery + one pipeline for /packages (ADR-0076 D11 step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OQ#9 flagged discovery and packages as the two genuinely double-mounted domains between the REST server and the dispatcher bridge, with first-registration-wins deciding which implementation a client saw. - kernel: new public hasPlugin(name) — plugins can make composition- dependent decisions deterministically during their own start(). - dispatcher bridge: cede ${prefix}/discovery to com.objectstack.rest.api when that plugin is registered; keep it only as the fallback owner in REST-less compositions. /.well-known/objectstack stays dispatcher-owned (no other registrant). - dispatcher bridge: the /packages route family now flows through dispatch() (like analytics/i18n/automation/AI) instead of calling handlePackages() directly, so the explicit mounts and the cloud hosts' @objectstack/hono catch-all share one per-request pipeline (kernel resolution, identity, auth gate). The direct path also dropped req.query on several routes — the documented ?overwrite=true install flag never reached the handler over the mounted routes. - ADR-0076 OQ#9: record step 2 as shipped and re-scope step 1 with the cross-repo finding — the 'dead' dispatch() branches are the cloud hosts' fallback fabric via the createHonoApp catch-all (/share-links is the designed primary surface for per-env kernels; /notifications has no other HTTP owner), so killing them waits for per-domain handlers (step 3). Refs #2462 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01A1BtxNeUaGmvUYr8FwtUNu --- docs/adr/0076-objectql-core-tiering.md | 4 +- packages/core/src/kernel.ts | 14 + .../src/dispatcher-plugin.routes.test.ts | 18 ++ packages/runtime/src/dispatcher-plugin.ts | 255 ++++++------------ 4 files changed, 112 insertions(+), 179 deletions(-) diff --git a/docs/adr/0076-objectql-core-tiering.md b/docs/adr/0076-objectql-core-tiering.md index 36ee0e5500..a92b9fe390 100644 --- a/docs/adr/0076-objectql-core-tiering.md +++ b/docs/adr/0076-objectql-core-tiering.md @@ -204,6 +204,8 @@ import { SecurityPlugin } from '@objectstack/plugin-security'; 6. **Data-facade home** — does the `DataProtocol` impl live in the engine-adjacent transport layer / `rest`, or a small `@objectstack/protocol-data`? (It is thin and transport-shaped.) 7. **Metadata package name (post-segmentation)** — **Resolved: keep `@objectstack/metadata-protocol`** (already published; renaming has ~0 benefit and real churn). The `protocol` suffix is a low-cost naming exception — the contract is in `@objectstack/spec/api`; a README note in the package should clarify impl-vs-contract. 8. **Per-domain versioning** — once segmented, do capability protocols get independent version markers / a `getCapabilities()` discovery method? -9. **dispatcher vs rest-server overlap** — **Resolved (audit, #2462): delineate, do not merge.** They partition the API surface by domain rather than duplicating it: in a standard boot REST owns data/meta/batch/import-export/search/email/forms/sharing/reports/approvals/security, the dispatcher bridge owns mcp/ai/graphql/keys/automation/actions/storage/i18n/analytics/health/ready. Only **discovery** and **packages** are genuinely double-mounted (first-registration-wins — the D11 smell to fix), and the dispatcher's `/data`/`/meta`/`/ui`/`/notifications`/`/share-links`/`/security` `dispatch()` branches are **not mounted** in a standard boot (they are live only when `HttpDispatcher` is used directly as a callable by out-of-tree adapters). D11 worklist: (1) kill or delegate the dead callable branches; (2) give packages + discovery a single owner; (3) extract per-domain handlers; (4) unify env-resolution on the injected `KernelResolver` seam. +9. **dispatcher vs rest-server overlap** — **Resolved (audit, #2462): delineate, do not merge.** They partition the API surface by domain rather than duplicating it: in a standard boot REST owns data/meta/batch/import-export/search/email/forms/sharing/reports/approvals/security, the dispatcher bridge owns mcp/ai/graphql/keys/automation/actions/storage/i18n/analytics/health/ready. Only **discovery** and **packages** were genuinely double-mounted (first-registration-wins — the D11 smell), and the dispatcher's `/data`/`/meta`/`/ui`/`/notifications`/`/share-links`/`/security` `dispatch()` branches are **not mounted** in a standard boot. D11 worklist: (1) kill or delegate the dead callable branches; (2) give packages + discovery a single owner; (3) extract per-domain handlers; (4) unify env-resolution on the injected `KernelResolver` seam. + - **(2) shipped**: `${prefix}/discovery` has a single deterministic owner — the dispatcher bridge cedes it to `com.objectstack.rest.api` when that plugin is registered (`kernel.hasPlugin`, new public API), keeping it only as the fallback in REST-less compositions; `/.well-known/objectstack` stays dispatcher-owned. The bridge's `/packages` family now flows through `dispatch()` (like analytics/i18n/automation/AI) instead of calling `handlePackages()` directly, so both HTTP entries into the domain (explicit mounts + the cloud hosts' catch-all) share one pipeline — the direct path used to skip kernel/identity resolution and drop `req.query` (the documented `?overwrite=true` install flag never worked over the mounted routes). + - **(1) re-scoped (cross-repo finding)**: the "dead" branches are NOT safely killable — the `@objectstack/hono` `createHonoApp()` catch-all delegates every unmatched `${prefix}/*` to `dispatch()`, and both cloud hosts mount it under their plugin routes. In that composition `/share-links` is the **designed primary surface** (per-env kernels register the `shareLinks` service with `registerShareLinkRoutes:false`; the host dispatcher serves the route after kernel swap — documented in cloud's `artifact-kernel-factory` and `default-environment-plugins`), and `/notifications` has **no other HTTP owner anywhere**. `/data`/`/meta`/`/ui`/`/security` are REST-shadowed but still catch REST misses. Killing therefore waits until the catch-all is retired in favor of per-domain handlers (step 3); until then the branches are the cloud fallback fabric, not dead code. 10. **Validate multi-adapter** — **Resolved (validated, #2462): the port is free of hard Hono-isms.** A zero-dependency `node:http` reference adapter (`@objectstack/http-conformance`, private QA gate under `packages/qa/`) runs the dispatcher bridge + REST generator unchanged; a cross-adapter conformance suite (40 assertions incl. `/data` CRUD roundtrip, `:param` routing, 404/405 semantics, SSE streaming, discovery) passes identically on both adapters. Findings: the Hono adapter's Host-header backfill is adapter-local (a Fetch-API artifact, not a port leak); all remaining Hono coupling is confined to the `getRawApp()` escape hatch (metadata HMR, cloud-connection/marketplace routes, static/SPA + CORS + Server-Timing), whose consumers feature-detect and degrade. Follow-up for D11: codify the two soft extensions consumers already rely on (`res.write`/`res.end` for SSE, `getPort()`) and 404/405 semantics into the `IHttpServer` contract. 11. **D12 stub marker** — **Resolved (#2462): standard `__serviceInfo` self-descriptor** (`ServiceSelfInfoSchema` in `spec/api/discovery.zod.ts`, read via `readServiceSelfInfo()`), with plugin-dev's legacy `_dev: true` normalized to `{ status: 'stub', handlerReady: false }`. Both discovery builders honor it; the analytics fallback self-identifies as `degraded`. diff --git a/packages/core/src/kernel.ts b/packages/core/src/kernel.ts index c1c552c032..7d6f7c080c 100644 --- a/packages/core/src/kernel.ts +++ b/packages/core/src/kernel.ts @@ -461,6 +461,20 @@ export class ObjectKernel { return new Map(this.pluginStartTimes); } + /** + * Whether a plugin with the given name has been registered on this kernel. + * + * Registration happens synchronously in `use()` before any plugin's + * `start()` runs, so a plugin may use this during its own start() to make + * composition-dependent decisions deterministically — e.g. the dispatcher + * bridge cedes `${prefix}/discovery` to `com.objectstack.rest.api` when + * both are mounted (ADR-0076 D11: single owner per route, not + * first-registration-wins). + */ + hasPlugin(name: string): boolean { + return this.plugins.has(name); + } + /** * Get a service (sync helper) */ diff --git a/packages/runtime/src/dispatcher-plugin.routes.test.ts b/packages/runtime/src/dispatcher-plugin.routes.test.ts index 14635abb91..adfb0f839a 100644 --- a/packages/runtime/src/dispatcher-plugin.routes.test.ts +++ b/packages/runtime/src/dispatcher-plugin.routes.test.ts @@ -126,4 +126,22 @@ describe('createDispatcherPlugin — HTTP route registration', () => { expect(headers['Cache-Control'], `${route} Cache-Control`).toBe('no-store'); } }); + + // ADR-0076 D11 / OQ#9 — single owner for ${prefix}/discovery. When the REST + // plugin is registered on the same kernel it serves /api/v1/discovery itself; + // which payload a client saw used to depend on plugin start order + // (first-registration-wins). The bridge must cede the route deterministically + // and keep /.well-known/objectstack (dispatcher-owned, no other registrant). + it('cedes /api/v1/discovery to the REST plugin when it is registered', async () => { + const { server, routes } = makeFakeServer(); + const ctx = makeCtx(server); + ctx.getKernel().hasPlugin = (name: string) => name === 'com.objectstack.rest.api'; + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.(ctx); + + expect(routes).not.toContain('GET /api/v1/discovery'); + expect(routes).toContain('GET /.well-known/objectstack'); + // Non-discovery routes are unaffected by the cession. + expect(routes).toContain('GET /api/v1/packages'); + }); }); diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 7d8c2b765d..9a2916e3e2 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -527,17 +527,33 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu }); // ── Discovery (versioned API path) ────────────────────────── - server.get(`${prefix}/discovery`, async (_req: any, res: any) => { - if (securityHeaders) { - for (const [k, v] of Object.entries(securityHeaders)) { - res.header(k, v); + // Single owner (ADR-0076 D11 / OQ#9): when the REST plugin is + // mounted on the same kernel it registers `${prefix}/discovery` + // itself (rest-server registerDiscoveryEndpoints), and which + // payload a client saw used to depend on plugin start order + // (first-registration-wins on Hono). Cede the route to REST + // deterministically; this bridge registers it only as the + // fallback owner in REST-less compositions. `/.well-known/ + // objectstack` above stays dispatcher-owned unconditionally — + // no other plugin registers it. + const restRegistered = + typeof (kernel as { hasPlugin?: (n: string) => boolean }).hasPlugin === 'function' && + (kernel as { hasPlugin: (n: string) => boolean }).hasPlugin('com.objectstack.rest.api'); + if (!restRegistered) { + server.get(`${prefix}/discovery`, async (_req: any, res: any) => { + if (securityHeaders) { + for (const [k, v] of Object.entries(securityHeaders)) { + res.header(k, v); + } } - } - // See the .well-known handler above: discovery must not be cached - // (mutable runtime config; cloud#152 stale `routes.mcp`). - res.header('Cache-Control', 'no-store'); - res.json({ data: await dispatcher.getDiscoveryInfo(prefix) }); - }); + // See the .well-known handler above: discovery must not be cached + // (mutable runtime config; cloud#152 stale `routes.mcp`). + res.header('Cache-Control', 'no-store'); + res.json({ data: await dispatcher.getDiscoveryInfo(prefix) }); + }); + } else { + ctx.logger.info(`[Dispatcher] ${prefix}/discovery ceded to com.objectstack.rest.api (single owner)`); + } // ── Health ────────────────────────────────────────────────── server.get(`${prefix}/health`, async (_req: any, res: any) => { @@ -676,178 +692,61 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu }); // ── Packages ──────────────────────────────────────────────── - server.get(`${prefix}/packages`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages('', 'GET', {}, req.query, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages('', 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.get(`${prefix}/packages/:id/export`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/export`, 'GET', {}, req.query, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.get(`${prefix}/packages/:id`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}`, 'GET', {}, req.query, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.delete(`${prefix}/packages/:id`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}`, 'DELETE', {}, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - // Edit a package's manifest (name / description / version). The - // handler for this has existed in handlePackages all along, but the - // route was never registered, so PATCH /packages/:id 405'd and the - // Studio "edit package" form silently failed. `/:id` is a single - // segment, so this does not shadow the `/:id/enable|disable` routes. - server.patch(`${prefix}/packages/:id`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}`, 'PATCH', req.body, req.query, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.patch(`${prefix}/packages/:id/enable`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/enable`, 'PATCH', {}, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.patch(`${prefix}/packages/:id/disable`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/disable`, 'PATCH', {}, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages/:id/publish`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/publish`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); + // Single pipeline (ADR-0076 D11 / OQ#9): every package route flows + // through dispatch() — like analytics / i18n / automation / AI — + // so both HTTP entries into this domain (these explicit mounts and + // the @objectstack/hono catch-all the cloud hosts mount underneath) + // run the SAME per-request pipeline: kernel resolution, identity + // resolution, auth gate. These used to call handlePackages() + // 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); + } + }); + }; + 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. - server.post(`${prefix}/packages/:id/publish-drafts`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/publish-drafts`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages/:id/revert`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/revert`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - // The dispatcher's handlePackages grew branches that were never - // mounted here, so they 404'd at the HTTP layer on every serve - // stack (hono notFound) despite working handler code: duplicate - // (ADR-0070 D4), adopt-orphans (D5), discard-drafts, and the - // ADR-0067 commit-history / rollback family. Mount them all — - // the handlers already exist; this is routing only. - server.post(`${prefix}/packages/:id/duplicate`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/duplicate`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages/:id/adopt-orphans`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/adopt-orphans`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages/:id/discard-drafts`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/discard-drafts`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.get(`${prefix}/packages/:id/commits`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/commits`, 'GET', undefined, req.query ?? {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages/:id/commits/:commitId/revert`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages( - `/${req.params.id}/commits/${req.params.commitId}/revert`, - 'POST', - req.body, - {}, - { request: req }, - ); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); - - server.post(`${prefix}/packages/:id/rollback`, async (req: any, res: any) => { - try { - const result = await dispatcher.handlePackages(`/${req.params.id}/rollback`, 'POST', req.body, {}, { request: req }); - sendResult(result, res); - } catch (err: any) { - errorResponse(err, res); - } - }); + 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`); // ── Storage ───────────────────────────────────────────────── server.post(`${prefix}/storage/upload`, async (req: any, res: any) => {