From ca829ddcfd15cccfdf760fcb7d270e02ecaf12ca Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 10:43:36 +0000 Subject: [PATCH] fix(cloud-connection): derive features.marketplace from what is mounted (#8356) RuntimeConfigPlugin built its /api/v1/runtime/config response with `marketplace: true` as a literal, so every runtime mounting the plugin told the Console the catalog was browsable -- including runtimes where MarketplaceProxyPlugin was never mounted because no control plane resolved. Same declared-is-not-enforced shape as #8343, one key over. The flag is now read per request off the route table of the app serving the response. Measured rather than assumed: the proxy registers no service (its init says so), and IHttpServer.getMountedRoutes/resolveMountedRoute exclude framework-native getRawApp() mounts by construction -- so the raw app's own route ledger is the only honest seam, and it is also the one that covers the cloud control plane, which serves the catalog natively with no proxy. install-local routes are excluded from the derivation: they are the offline half, mounted exactly on the runtimes with no catalog. Both directions pinned, with the positive case mounting the REAL proxy on the shared app rather than copying its route string. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj --- .../runtime-config-marketplace-derived.md | 48 ++++ ...time-config-marketplace-derivation.test.ts | 224 ++++++++++++++++++ .../src/runtime-config-plugin.test.ts | 20 +- .../src/runtime-config-plugin.ts | 124 +++++++++- 4 files changed, 411 insertions(+), 5 deletions(-) create mode 100644 .changeset/runtime-config-marketplace-derived.md create mode 100644 packages/cloud-connection/src/runtime-config-marketplace-derivation.test.ts diff --git a/.changeset/runtime-config-marketplace-derived.md b/.changeset/runtime-config-marketplace-derived.md new file mode 100644 index 0000000000..8050c810dd --- /dev/null +++ b/.changeset/runtime-config-marketplace-derived.md @@ -0,0 +1,48 @@ +--- +"@objectstack/cloud-connection": patch +--- + +fix(cloud-connection): `features.marketplace` is derived from what is mounted, not hardcoded `true` (#8356) + +`GET /api/v1/runtime/config` built its response with `marketplace: true` as a +literal, so **every** runtime that mounted `RuntimeConfigPlugin` told the Console +the package catalog was browsable — including runtimes where +`MarketplaceProxyPlugin` was never mounted because no control plane resolved. The +SPA rendered a browse affordance the runtime could not serve. That is the same +declared-is-not-enforced shape as #8343, one key over. + +It was a live constraint, not a hypothetical: it is why #8343 mounts install-local +**alone** on a cloud-less runtime and deliberately does not also mount +`RuntimeConfigPlugin` there. Doing so would have restored the Console's knowledge +of install-local at the cost of asserting a browse capability that is definitively +absent — trading the reported bug for its mirror image. + +The flag is now **observed** per request, off the route table of the app serving +the response: `true` when a marketplace browse surface is mounted on it, `false` +when none is. `/api/v1/marketplace/install-local` is deliberately excluded — it is +the offline install half, mounted precisely on the runtimes that have no catalog, +and counting it as browse would recreate the defect one key over. + +Reading the route table rather than a proxy-specific signal is what makes one +derivation true for every distribution: `MarketplaceProxyPlugin` registers no +service (it announces itself only by mounting its routes), the `IHttpServer` +mount-introspection members exclude framework-native `getRawApp()` mounts by +construction, and the ObjectStack Cloud control plane serves the catalog +**natively** with no proxy at all. The route table is the union that covers all +three. + +**What changes for hosts.** A runtime that mounts a marketplace browse surface +reports exactly what it did before. A runtime that mounts none now reports +`marketplace: false` instead of `true` — the correction — and its Console stops +offering catalog browse it cannot serve. A cloud-less runtime can therefore report +`installLocal: true` truthfully without also claiming browse. No config knob was +added: a knob would repeat one layer up the every-host-must-remember failure that +propagated the original defect into the self-hosted EE image, where both the host +config and this package's README kept a hand-maintained flag out of step with +their own mounting. + +**Escape hatch, unchanged.** The derivation is the base value, not a veto: the +open-core `resolveFeatures` seam still merges over it, so a host on an adapter +whose raw app exposes no route ledger (where the flag conservatively reports +`false`, with a warning logged at mount time) can still declare the capability it +knows it serves. diff --git a/packages/cloud-connection/src/runtime-config-marketplace-derivation.test.ts b/packages/cloud-connection/src/runtime-config-marketplace-derivation.test.ts new file mode 100644 index 0000000000..bb74593813 --- /dev/null +++ b/packages/cloud-connection/src/runtime-config-marketplace-derivation.test.ts @@ -0,0 +1,224 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `features.marketplace` is an OBSERVATION, not a constant (#8356). + * + * It shipped as the literal `true`, so `/api/v1/runtime/config` told the + * Console the catalog was browsable on every runtime that mounted + * RuntimeConfigPlugin — including one where `MarketplaceProxyPlugin` was never + * mounted because no control plane resolved. The SPA rendered a browse + * affordance the runtime could not serve. That is #8343's declared-is-not- + * enforced shape one key over, and it is why #8343 mounts install-local ALONE + * on a cloud-less runtime: reporting install-local truthfully would have cost + * a false browse claim. + * + * ## Why the negative direction is the whole point of this file + * + * A suite that only asserts the mounted case passes unchanged against the + * hardcoded `true` it is meant to replace — it proves nothing about the very + * defect. Every case here that expects `false` is load-bearing: revert the + * derivation to `marketplace: true` and each one fails, while the positive + * cases stay green. + * + * ## Why the fixtures are shaped the way they are + * + * The fake raw app mirrors what `getRawApp()` really hands back — a Hono + * instance whose public `routes` array collects EVERY registration, verb + * methods and `use()`/`all()` alike, framework-native mounts included. + * Measured against hono@4.12.34: + * + * app.use('/api/v1/*', mw) → { method: 'ALL', path: '/api/v1/*' } + * app.all('/api/v1/marketplace/*', h) → { method: 'ALL', path: '/api/v1/marketplace/*' } + * app.post('/api/v1/marketplace/install-local', h) + * → { method: 'POST', path: '/api/v1/marketplace/install-local' } + * + * And the sibling plugins are the REAL ones, started against the REAL shared + * app, rather than their route strings copied in here. That is deliberate: the + * derivation keys on a path prefix the proxy owns, and a hand-spelled fixture + * would keep agreeing with a stale copy of that spelling long after the proxy + * changed it — the flag would flip to `false` in production with this suite + * still green. + */ + +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { RuntimeConfigPlugin, type RuntimeConfigPluginConfig } from './runtime-config-plugin.js'; +import { MarketplaceProxyPlugin } from './marketplace-proxy-plugin.js'; +import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js'; + +interface RouteRecord { method: string; path: string } + +interface HonoShapedApp { + routes: RouteRecord[]; + handlers: Map; + get(path: string, handler?: any): void; + post(path: string, handler?: any): void; + put(path: string, handler?: any): void; + delete(path: string, handler?: any): void; + head(path: string, handler?: any): void; + all(path: string, handler?: any): void; + use(path: string, handler?: any): void; +} + +/** A raw app shaped like the Hono instance `getRawApp()` returns. */ +function createApp(): HonoShapedApp { + const routes: RouteRecord[] = []; + const handlers = new Map(); + const record = (method: string) => (path: string, handler?: any) => { + routes.push({ method, path }); + handlers.set(`${method} ${path}`, handler); + }; + return { + routes, + handlers, + get: record('GET'), + post: record('POST'), + put: record('PUT'), + delete: record('DELETE'), + head: record('HEAD'), + all: record('ALL'), + // Hono files middleware into the same ledger, under ALL. + use: record('ALL'), + }; +} + +/** Start a plugin against `app` and fire its `kernel:ready` hooks. */ +async function startOn(app: unknown, plugin: { start(ctx: any): Promise }): Promise { + const hooks: Array<() => Promise> = []; + const warnings: string[] = []; + const services: Record = { + 'http.server': { getRawApp: () => app }, + manifest: { register() {} }, + auth: { api: { getSession: async () => ({ user: { id: 'admin' } }) } }, + objectql: { syncSchemas: async () => undefined }, + }; + const ctx: any = { + logger: { info() {}, warn: (m: unknown) => { warnings.push(String(m)); }, error() {} }, + getService: (name: string) => { + const svc = services[name]; + if (svc === undefined) throw new Error(`no ${name}`); + return svc; + }, + hook: (_event: string, cb: () => Promise) => { hooks.push(cb); }, + }; + await plugin.start(ctx); + for (const cb of hooks) await cb(); + return warnings; +} + +/** Ask the mounted `/api/v1/runtime/config` handler for its payload. */ +async function readConfig(app: HonoShapedApp, host = ''): Promise { + const handler = app.handlers.get('GET /api/v1/runtime/config'); + if (typeof handler !== 'function') throw new Error('runtime/config was never mounted'); + return handler({ + req: { header: (n: string) => (n.toLowerCase() === 'host' ? host : undefined) }, + json: (body: any) => body, + }); +} + +function runtimeConfig(config: RuntimeConfigPluginConfig = {}): RuntimeConfigPlugin { + return new RuntimeConfigPlugin({ controlPlaneUrl: '', singleEnvironment: true, ...config }); +} + +/** A temp ledger dir so the real install-local plugin touches no shared state. */ +function tempStorageDir(): string { + return mkdtempSync(join(tmpdir(), 'os-8356-')); +} + +describe('features.marketplace — mounted proxy reports true', () => { + it('reports true when the REAL MarketplaceProxyPlugin is mounted on the same app', async () => { + const app = createApp(); + await startOn(app, new MarketplaceProxyPlugin({ controlPlaneUrl: 'http://cloud.test', cacheDisabled: true })); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { marketplace: true } }); + }); + + it('is read per request, so a proxy mounted AFTER this plugin still counts', async () => { + // Plugin start() order across kernel:ready hooks is not guaranteed, so + // a mount-time snapshot would report whichever hook happened to run + // first. The handler runs long after every hook — that is the seam. + const app = createApp(); + await startOn(app, runtimeConfig()); + expect(await readConfig(app)).toMatchObject({ features: { marketplace: false } }); + + await startOn(app, new MarketplaceProxyPlugin({ controlPlaneUrl: 'http://cloud.test', cacheDisabled: true })); + expect(await readConfig(app)).toMatchObject({ features: { marketplace: true } }); + }); + + it('counts a NATIVE browse mount this package never installed', async () => { + // The cloud control plane serves /api/v1/marketplace/packages from its + // own route module with no proxy anywhere; the flag's meaning there is + // already "reachable (proxy or native)". A proxy-specific signal would + // report false on the one deployment that definitely has a catalog. + const app = createApp(); + app.get('/api/v1/marketplace/packages', () => {}); + app.get('/api/v1/marketplace/packages/:id', () => {}); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { marketplace: true } }); + }); +}); + +describe('features.marketplace — no browse surface reports false', () => { + it('reports false on a runtime with no marketplace mount at all', async () => { + const app = createApp(); + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { marketplace: false } }); + }); + + it('the cloud-less runtime reports installLocal truthfully WITHOUT claiming browse', async () => { + // #8356's acceptance, and the reason #8343 could not mount this plugin + // on an air-gapped runtime: the REAL install-local plugin mounted alone + // must give the Console install-local and nothing else. + const dir = tempStorageDir(); + try { + const app = createApp(); + await startOn(app, new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir })); + await startOn(app, runtimeConfig({ installLocal: true })); + + const body = await readConfig(app); + expect(body.features.installLocal).toBe(true); + expect(body.features.marketplace).toBe(false); + // The fixture is only meaningful if install-local really did mount. + expect(app.routes.some((r) => r.path.startsWith('/api/v1/marketplace/install-local'))).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('broad middleware and an SPA catch-all are not evidence of a catalog', async () => { + const app = createApp(); + app.use('/api/v1/*', () => {}); + app.get('/*', () => {}); + app.get('/api/v1/marketplaceish/packages', () => {}); // adjacent namespace + await startOn(app, runtimeConfig()); + + expect(await readConfig(app)).toMatchObject({ features: { marketplace: false } }); + }); + + it('reports false — and warns once — when the raw app exposes no route table', async () => { + // Do not claim a capability you could not verify; claiming it + // unverified is the defect. The warning makes the downgrade traceable. + const opaqueApp: any = { + handlers: new Map(), + get(path: string, handler?: any) { this.handlers.set(`GET ${path}`, handler); }, + }; + const warnings = await startOn(opaqueApp, runtimeConfig()); + + expect(await readConfig(opaqueApp)).toMatchObject({ features: { marketplace: false } }); + expect(warnings.filter((w) => w.includes('no route table'))).toHaveLength(1); + }); + + it('a host that knows better can still declare it through resolveFeatures', async () => { + // The open-core seam is unchanged: the derivation is the BASE value, + // not a veto, so an exotic adapter is not a dead end. + const app = createApp(); + await startOn(app, runtimeConfig({ resolveFeatures: () => ({ marketplace: true }) })); + + expect(await readConfig(app)).toMatchObject({ features: { marketplace: true } }); + }); +}); diff --git a/packages/cloud-connection/src/runtime-config-plugin.test.ts b/packages/cloud-connection/src/runtime-config-plugin.test.ts index dc305e6808..03a8fb4cfc 100644 --- a/packages/cloud-connection/src/runtime-config-plugin.test.ts +++ b/packages/cloud-connection/src/runtime-config-plugin.test.ts @@ -18,7 +18,16 @@ async function getConfig(opts: { host?: string; }): Promise { let handler: ((c: any) => Promise) | undefined; - const rawApp = { get(path: string, h: (c: any) => Promise) { if (path === '/api/v1/runtime/config') handler = h; } }; + // Hono-shaped: a real `routes` ledger, empty because this suite mounts no + // marketplace surface — so `features.marketplace` derives to false here. + // The derivation itself is pinned in runtime-config-marketplace-derivation.test.ts. + const rawApp = { + routes: [] as Array<{ method: string; path: string }>, + get(path: string, h: (c: any) => Promise) { + this.routes.push({ method: 'GET', path }); + if (path === '/api/v1/runtime/config') handler = h; + }, + }; const services: Record = { 'http-server': { getRawApp: () => rawApp } }; if (opts.resolveByHostname) services['env-registry'] = { resolveByHostname: opts.resolveByHostname }; const ctx: any = { @@ -38,7 +47,9 @@ describe('RuntimeConfigPlugin feature seam', () => { it('always ships the base mechanism flags', async () => { const body = await getConfig({}); expect(body.features.installLocal).toBe(false); - expect(body.features.marketplace).toBe(true); + // Derived, not declared (#8356): no browse surface is mounted on this + // app, so the mechanism must not claim one. + expect(body.features.marketplace).toBe(false); expect(body.features.aiStudio).toBe(true); expect(body.features.autoPublishAiBuilds).toBe(false); }); @@ -58,8 +69,9 @@ describe('RuntimeConfigPlugin feature seam', () => { expect(body.features.customDomain).toBe(true); expect(body.features.sso).toBe(false); expect(body.features.aiStudio).toBe(true); - // base flags survive the merge - expect(body.features.marketplace).toBe(true); + // base flags survive the merge — including the derived one, which this + // host's hook does not name (nothing browse-shaped is mounted here). + expect(body.features.marketplace).toBe(false); }); it('honours the deprecated resolvePlanFeatures alias', async () => { diff --git a/packages/cloud-connection/src/runtime-config-plugin.ts b/packages/cloud-connection/src/runtime-config-plugin.ts index e00461c9c9..8b878abef7 100644 --- a/packages/cloud-connection/src/runtime-config-plugin.ts +++ b/packages/cloud-connection/src/runtime-config-plugin.ts @@ -37,6 +37,25 @@ * `aiStudio` / `autoPublishAiBuilds` are the framework's own non-commercial * mechanism defaults (ADR-0005: AI authoring is an all-plan capability gated * by cost, not a paid tier), so they keep first-class config knobs here. + * + * ## `features.marketplace` is DERIVED, not declared (#8356) + * + * It used to be the literal `true`, so every runtime mounting this plugin told + * the Console the catalog was browsable — including one where + * `MarketplaceProxyPlugin` was never mounted because no control plane + * resolved. The affordance rendered; the runtime could not serve it. That is + * the same declared-is-not-enforced shape as #8343, one key over, and it is + * why #8343 mounts install-local ALONE on a cloud-less runtime rather than + * also mounting this plugin: reporting install-local truthfully would have + * cost a false browse claim. + * + * The flag is now read off what is really mounted — see + * {@link hasMarketplaceBrowseMount} for the seam and why it is that one. A + * config knob was rejected on the record (#8343 ACCEPT, 2026-08-13): it + * repeats one layer up the every-host-must-remember failure that propagated + * the original defect into the EE image, where both the host config and this + * package's README kept a hand-maintained flag out of step with their own + * mounting. */ import type { Plugin, PluginContext } from '@objectstack/core'; @@ -53,6 +72,92 @@ interface EnvRegistrySurface { resolveHostname?(hostname: string): Promise; } +/** + * The marketplace HTTP namespace, and the one sub-path inside it that is NOT + * a browse surface. + * + * These literals are deliberately NOT imported from `marketplace-proxy-plugin` + * — the derivation must also see a browse surface this package never mounted + * (see {@link hasMarketplaceBrowseMount}), so keying it on the proxy module + * would narrow it back to one provider. The coupling to the proxy's own + * spelling is instead pinned by test: the positive direction mounts the REAL + * `MarketplaceProxyPlugin` onto the same app rather than hand-spelling its + * route, so a change to its prefix fails here rather than silently flipping + * this flag to `false`. + */ +const MARKETPLACE_API_PREFIX = '/api/v1/marketplace'; +const MARKETPLACE_INSTALL_LOCAL_PREFIX = `${MARKETPLACE_API_PREFIX}/install-local`; + +/** + * Does this registered route pattern mount a marketplace BROWSE surface? + * + * `/api/v1/marketplace/install-local` is excluded on purpose. It is the + * offline install half, mounted precisely on the runtimes that have no + * catalog, and counting it as browse would recreate this bug's mirror image: + * #8343's cloud-less deployment reporting a capability whose route 404s. + * Patterns outside the namespace (`/api/v1/*` middleware, an SPA `/*` + * catch-all) are not evidence of anything and never match. + */ +function isMarketplaceBrowsePattern(pattern: string): boolean { + if (pattern.startsWith(MARKETPLACE_INSTALL_LOCAL_PREFIX)) return false; + if (!pattern.startsWith(MARKETPLACE_API_PREFIX)) return false; + // `/api/v1/marketplaceish/...` is somebody else's namespace. + const rest = pattern.slice(MARKETPLACE_API_PREFIX.length); + return rest === '' || rest.startsWith('/'); +} + +/** + * Is a marketplace browse surface actually mounted on the app serving this + * response? (#8356) + * + * ## Why the raw app's route table, and not any of the alternatives + * + * Measured on `main`, not assumed — the card proposed reading a kernel + * registration and there is none: + * + * - **`MarketplaceProxyPlugin` registers no service.** Its `init` says so in + * as many words ("No services registered — pure HTTP wiring during + * `start()`"); it announces itself only by mounting + * `${MARKETPLACE_API_PREFIX}/*` on the raw app. So there is nothing on the + * kernel to look up, and adding a registration purely to read it back + * would be a mechanism invented for its own observation rather than a fix. + * - **`IHttpServer.getMountedRoutes()` / `resolveMountedRoute()` cannot see + * it.** Both are scoped to routes registered through the adapter's own + * verb methods — "routes an adapter mounts on its framework-native handle + * behind `getRawApp` are outside this table by construction" (the contract's + * own words, `packages/spec/src/contracts/http-server.ts`), and + * `resolveMountedRoute` filters the live router's verdict back through that + * same ledger. The proxy mounts through `getRawApp()`, so the adapter + * ledger reports nothing for it. + * - **A proxy-specific signal would under-report.** The ObjectStack Cloud + * control plane serves `/api/v1/marketplace/packages*` NATIVELY (its own + * route module), with no proxy anywhere; the flag's documented meaning in + * that distribution is already "`/api/v1/marketplace/*` is reachable + * (proxy or native)". Reading the route table is what makes one derivation + * true for both. + * + * The raw app's route ledger is the union of everything registered on it — + * adapter verb methods and framework-native `getRawApp()` mounts alike — so it + * answers exactly the question the flag claims to answer. Read per request: + * plugin `start()` order across `kernel:ready` hooks is not guaranteed, and by + * request time every hook has run. + * + * ## When it cannot be observed + * + * An adapter whose raw app exposes no route ledger returns `false` — do not + * claim a capability you could not verify; claiming it unverified is the + * defect. A host on such an adapter that KNOWS browse is live states it + * through the `resolveFeatures` seam, which still merges over this base. + */ +function hasMarketplaceBrowseMount(rawApp: unknown): boolean { + const routes = (rawApp as { routes?: unknown } | null | undefined)?.routes; + if (!Array.isArray(routes)) return false; + return routes.some((route) => { + const pattern = (route as { path?: unknown } | null | undefined)?.path; + return typeof pattern === 'string' && isMarketplaceBrowsePattern(pattern); + }); +} + /** * Feature-flag overrides a host's distribution policy can derive per request. @@ -202,6 +307,19 @@ export class RuntimeConfigPlugin implements Plugin { } const rawApp = httpServer.getRawApp(); + // Diagnosable once at mount time rather than per request: an + // adapter with no observable route ledger makes + // `features.marketplace` report false for the whole process, and + // a silently downgraded capability flag is hard to trace from the + // SPA end. See hasMarketplaceBrowseMount(). + if (!Array.isArray((rawApp as { routes?: unknown } | null | undefined)?.routes)) { + ctx.logger?.warn?.( + '[RuntimeConfigPlugin] raw app exposes no route table — features.marketplace will report false ' + + '(a mounted browse surface cannot be observed here). Declare it via resolveFeatures if this ' + + 'runtime does serve marketplace browse.', + ); + } + // A multi-tenant runtime serves many subdomains, each mapped to // one environment. Telling the SPA *which* environment it is // attached to (per-request) lets the App Marketplace skip the @@ -286,7 +404,11 @@ export class RuntimeConfigPlugin implements Plugin { defaultEnvironmentId, features: { installLocal: this.installLocal, - marketplace: true, + // Observed, not declared (#8356) — re-read per request + // because it is a property of the app, not of this + // plugin's config. A host's resolveFeatures still + // merges over it, same as every other base flag. + marketplace: hasMarketplaceBrowseMount(rawApp), // aiStudio + autoPublishAiBuilds + any distribution keys. ...features, },