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, },